From 24a1c8fc1eb4600a1a719ffbfcb20efd8998d72f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Wed, 13 May 2026 19:00:54 +0100 Subject: [PATCH 1/2] feat(ui): add mouse-drag text selection and clipboard copy - Drag-selection model (copySelection.ts) supports single-row, multi-row, double-click word expansion, and triple-click line selection, with column-level precision for both stack and split layouts - Selection highlight rendered at character-level granularity across diff rows, scoped to the active split side (left = A, right = B) - Triple-click in split mode automatically scopes to the clicked pane instead of selecting across both panes - Decorated copy preserves diff rails, line numbers, gutters, and file headers; code-only copy strips all decoration - B-side copy with decorations uses correct pane-local column offsets so selection, highlight, and clipboard agree - View > Copy decorations menu toggle (default off), controlled by copy_decorations config key - Pinned-header click extends selection into the stream body - Clipboard output via OSC 52 with transient status feedback --- CHANGELOG.md | 2 + src/core/config.test.ts | 2 + src/core/config.ts | 5 + src/core/loaders.ts | 1 + src/core/types.ts | 3 + src/ui/App.tsx | 44 +- src/ui/components/panes/AgentInlineNote.tsx | 2 +- src/ui/components/panes/DiffPane.tsx | 359 +++++++- src/ui/components/panes/DiffSection.tsx | 9 + src/ui/components/panes/copySelection.test.ts | 518 ++++++++++++ src/ui/components/panes/copySelection.ts | 563 +++++++++++++ src/ui/diff/PierreDiffView.tsx | 7 + src/ui/diff/pierre.test.ts | 115 +++ src/ui/diff/renderRows.tsx | 791 ++++++++++++++++-- src/ui/diff/rowStyle.ts | 28 + src/ui/diff/rowWindowing.test.ts | 2 + src/ui/lib/appMenus.ts | 10 + src/ui/lib/diffSectionGeometry.ts | 41 +- src/ui/lib/ui-lib.test.ts | 4 +- src/ui/themes.ts | 8 +- test/helpers/app-bootstrap.ts | 3 + 21 files changed, 2415 insertions(+), 102 deletions(-) create mode 100644 src/ui/components/panes/copySelection.test.ts create mode 100644 src/ui/components/panes/copySelection.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 89505edf..58dccbc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable user-visible changes to Hunk are documented in this file. ### Added +- Added mouse-drag text selection in diff views that copies selected rows to the system clipboard via OSC 52. A `View > Copy decorations` toggle (or `copy_decorations` config) controls whether the clipboard includes diff rails, gutters, and file headers or only the changed code. + ### Changed ### Fixed diff --git a/src/core/config.test.ts b/src/core/config.test.ts index f1e28080..817afcab 100644 --- a/src/core/config.test.ts +++ b/src/core/config.test.ts @@ -255,6 +255,7 @@ describe("config resolution", () => { "wrap_lines = true", "hunk_headers = false", "agent_notes = true", + "copy_decorations = false", ].join("\n"), ); @@ -280,6 +281,7 @@ describe("config resolution", () => { expect(bootstrap.initialWrapLines).toBe(true); expect(bootstrap.initialShowHunkHeaders).toBe(false); expect(bootstrap.initialShowAgentNotes).toBe(true); + expect(bootstrap.initialCopyDecorations).toBe(false); }); test("loadAppBootstrap exposes graphite when no theme is configured", async () => { diff --git a/src/core/config.ts b/src/core/config.ts index 21f12fd3..24438c93 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -15,6 +15,7 @@ const DEFAULT_VIEW_PREFERENCES: PersistedViewPreferences = { wrapLines: false, showHunkHeaders: true, showAgentNotes: false, + copyDecorations: true, }; interface ConfigResolutionOptions { @@ -63,6 +64,7 @@ function readConfigPreferences(source: Record): CommonOptions { wrapLines: normalizeBoolean(source.wrap_lines), hunkHeaders: normalizeBoolean(source.hunk_headers), agentNotes: normalizeBoolean(source.agent_notes), + copyDecorations: normalizeBoolean(source.copy_decorations), }; } @@ -81,6 +83,7 @@ function mergeOptions(base: CommonOptions, overrides: CommonOptions): CommonOpti wrapLines: overrides.wrapLines ?? base.wrapLines, hunkHeaders: overrides.hunkHeaders ?? base.hunkHeaders, agentNotes: overrides.agentNotes ?? base.agentNotes, + copyDecorations: overrides.copyDecorations ?? base.copyDecorations, }; } @@ -165,6 +168,7 @@ export function resolveConfiguredCliInput( wrapLines: DEFAULT_VIEW_PREFERENCES.wrapLines, hunkHeaders: DEFAULT_VIEW_PREFERENCES.showHunkHeaders, agentNotes: DEFAULT_VIEW_PREFERENCES.showAgentNotes, + copyDecorations: DEFAULT_VIEW_PREFERENCES.copyDecorations, }; if (userConfigPath) { @@ -194,6 +198,7 @@ export function resolveConfiguredCliInput( wrapLines: resolvedOptions.wrapLines ?? DEFAULT_VIEW_PREFERENCES.wrapLines, hunkHeaders: resolvedOptions.hunkHeaders ?? DEFAULT_VIEW_PREFERENCES.showHunkHeaders, agentNotes: resolvedOptions.agentNotes ?? DEFAULT_VIEW_PREFERENCES.showAgentNotes, + copyDecorations: resolvedOptions.copyDecorations ?? DEFAULT_VIEW_PREFERENCES.copyDecorations, }; return { diff --git a/src/core/loaders.ts b/src/core/loaders.ts index c53e6880..a6812386 100644 --- a/src/core/loaders.ts +++ b/src/core/loaders.ts @@ -1184,5 +1184,6 @@ export async function loadAppBootstrap( initialWrapLines: input.options.wrapLines ?? false, initialShowHunkHeaders: input.options.hunkHeaders ?? true, initialShowAgentNotes: input.options.agentNotes ?? false, + initialCopyDecorations: input.options.copyDecorations ?? false, }; } diff --git a/src/core/types.ts b/src/core/types.ts index 51442aaf..2def18dc 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -68,6 +68,7 @@ export interface CommonOptions { wrapLines?: boolean; hunkHeaders?: boolean; agentNotes?: boolean; + copyDecorations?: boolean; } export interface PersistedViewPreferences { @@ -77,6 +78,7 @@ export interface PersistedViewPreferences { wrapLines: boolean; showHunkHeaders: boolean; showAgentNotes: boolean; + copyDecorations: boolean; } export interface HelpCommandInput { @@ -281,4 +283,5 @@ export interface AppBootstrap { initialWrapLines?: boolean; initialShowHunkHeaders?: boolean; initialShowAgentNotes?: boolean; + initialCopyDecorations?: boolean; } diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 87dba698..7bc3e945 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -100,7 +100,9 @@ export function App({ const diffScrollRef = useRef(null); const wrapToggleScrollTopRef = useRef(null); const layoutToggleScrollTopRef = useRef(null); + const cancelCopySelectionRef = useRef<(() => void) | null>(null); const [layoutToggleRequestId, setLayoutToggleRequestId] = useState(0); + const [transientNoticeText, setTransientNoticeText] = useState(null); const [layoutMode, setLayoutMode] = useState(bootstrap.initialMode); const [themeId, setThemeId] = useState(() => bootstrap.initialTheme === "auto" @@ -110,6 +112,7 @@ export function App({ const [showAgentNotes, setShowAgentNotes] = useState(bootstrap.initialShowAgentNotes ?? false); const [showLineNumbers, setShowLineNumbers] = useState(bootstrap.initialShowLineNumbers ?? true); const [wrapLines, setWrapLines] = useState(bootstrap.initialWrapLines ?? false); + const [copyDecorations, setCopyDecorations] = useState(bootstrap.initialCopyDecorations ?? false); const [codeHorizontalOffset, setCodeHorizontalOffset] = useState(0); const [showHunkHeaders, setShowHunkHeaders] = useState(bootstrap.initialShowHunkHeaders ?? true); const [sidebarVisible, setSidebarVisible] = useState(() => !pagerMode); @@ -294,6 +297,20 @@ export function App({ setShowLineNumbers((current) => !current); }; + /** Toggle whether mouse selection copies review decorations or only file content. */ + const toggleCopyDecorations = () => { + setCopyDecorations((current) => !current); + }; + + // Show a short-lived status-bar message. Used to surface clipboard-copy outcomes that would + // otherwise be invisible to the user (OSC52 unsupported, etc.). + const showTransientNotice = useCallback((text: string, durationMs = 3000) => { + setTransientNoticeText(text); + setTimeout(() => { + setTransientNoticeText((current) => (current === text ? null : current)); + }, durationMs); + }, []); + /** Toggle whether diff code rows wrap instead of truncating to one terminal row. */ const toggleLineWrap = () => { // Capture the pre-toggle viewport position synchronously so DiffPane can restore the same @@ -483,11 +500,13 @@ export function App({ requestQuit, selectLayoutMode, selectThemeId: setThemeId, + copyDecorations, showAgentNotes, showHelp, showHunkHeaders, showLineNumbers, renderSidebar, + toggleCopyDecorations, toggleAgentNotes, toggleFocusArea, toggleHelp, @@ -500,6 +519,7 @@ export function App({ [ activeTheme.id, canRefreshCurrentInput, + copyDecorations, focusFilter, layoutMode, moveToAnnotatedFile, @@ -508,6 +528,7 @@ export function App({ review.moveToHunk, selectLayoutMode, triggerRefreshCurrentInput, + toggleCopyDecorations, showAgentNotes, showHelp, showHunkHeaders, @@ -627,6 +648,11 @@ export function App({ const diffHeaderStatsWidth = Math.min(24, Math.max(16, Math.floor(diffContentWidth / 3))); const diffHeaderLabelWidth = Math.max(8, diffContentWidth - diffHeaderStatsWidth - 1); const diffSeparatorWidth = Math.max(4, diffContentWidth - 2); + // Mirror the App layout: bodyPadding/2 left-padding, then sidebar + divider when visible. Keep + // this in lockstep with the body container's paddingLeft and the sidebar render branch below. + const diffPaneScreenLeft = + bodyPadding / 2 + (renderSidebar ? clampedSidebarWidth + DIVIDER_WIDTH : 0); + const diffPaneScreenTop = pagerMode ? 0 : 1; return ( { + endSidebarResize(event); + cancelCopySelectionRef.current?.(); + }} onMouseUp={(event) => { endSidebarResize(event); closeMenu(); + cancelCopySelectionRef.current?.(); }} > {renderSidebar ? ( @@ -700,10 +730,14 @@ export function App({ ) : null} { scrollCodeHorizontally(delta * FAST_CODE_HORIZONTAL_SCROLL_COLUMNS); }} + onCopyFeedback={showTransientNotice} onSelectFile={jumpToFile} onViewportCenteredHunkChange={(fileId, hunkIndex) => review.selectHunk(fileId, hunkIndex, { preserveViewport: true }) @@ -734,11 +769,14 @@ export function App({ /> - {!pagerMode && (focusArea === "filter" || Boolean(review.filter) || Boolean(noticeText)) ? ( + {!pagerMode && + (focusArea === "filter" || + Boolean(review.filter) || + Boolean(transientNoticeText ?? noticeText)) ? ( 1 ? `AI note ${noteIndex + 1}/${noteCount}` : "AI note"; } diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index dec8fd1c..9b0253f6 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -1,4 +1,8 @@ -import { type MouseEvent as TuiMouseEvent, type ScrollBoxRenderable } from "@opentui/core"; +import { + MouseButton, + type MouseEvent as TuiMouseEvent, + type ScrollBoxRenderable, +} from "@opentui/core"; import { useRenderer } from "@opentui/react"; import { useCallback, @@ -39,6 +43,20 @@ import { DiffSectionPlaceholder } from "./DiffSectionPlaceholder"; import { VerticalScrollbar, type VerticalScrollbarHandle } from "../scrollbar/VerticalScrollbar"; import type { VisibleBodyBounds } from "../../diff/rowWindowing"; import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff"; +import { + buildCopySelectedRowKeys, + clampCopyColumn, + copySelectionPointsEqual, + expandSelectionPoint, + findCopySelectionPoint, + normalizeCopySelectionRange, + renderCopySelectionText, + resolveCopySelectionSide, + type CopySelectionContext, + type CopySelectionDrag, + type CopySelectionPoint, + type CopySelectionSide, +} from "./copySelection"; const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; const EMPTY_VISIBLE_AGENT_NOTES_BY_FILE = new Map(); @@ -139,6 +157,9 @@ export function DiffPane({ scrollToNote = false, separatorWidth, pagerMode = false, + copyDecorations = false, + screenLeft = 0, + screenTop = 0, showAgentNotes, showLineNumbers, showHunkHeaders, @@ -150,7 +171,10 @@ export function DiffPane({ selectedHunkRevealRequestId, theme, width, + cancelCopySelectionRef, onOpenAgentNotesAtHunk, + onCopyFeedback, + onCopySelectionText, onScrollCodeHorizontally = () => {}, onSelectFile, onViewportCenteredHunkChange, @@ -167,6 +191,9 @@ export function DiffPane({ scrollToNote?: boolean; separatorWidth: number; pagerMode?: boolean; + copyDecorations?: boolean; + screenLeft?: number; + screenTop?: number; showAgentNotes: boolean; showLineNumbers: boolean; showHunkHeaders: boolean; @@ -178,7 +205,10 @@ export function DiffPane({ selectedHunkRevealRequestId?: number; theme: AppTheme; width: number; + cancelCopySelectionRef?: RefObject<(() => void) | null>; onOpenAgentNotesAtHunk: (fileId: string, hunkIndex: number) => void; + onCopyFeedback?: (text: string) => void; + onCopySelectionText?: (text: string) => void | boolean; onScrollCodeHorizontally?: (delta: number) => void; onSelectFile: (fileId: string) => void; onViewportCenteredHunkChange?: (fileId: string, hunkIndex: number) => void; @@ -275,6 +305,9 @@ export function DiffPane({ // other files can still use placeholders and viewport windowing. const windowingEnabled = !wrapLines; const [scrollViewport, setScrollViewport] = useState({ top: 0, height: 0 }); + const [copySelectionDrag, setCopySelectionDrag] = useState(null); + const lastClickTimeRef = useRef(0); + const clickCountRef = useRef(0); const scrollbarRef = useRef(null); const prevScrollTopRef = useRef(0); const previousSectionGeometryRef = useRef(null); @@ -490,6 +523,293 @@ export function DiffPane({ ); const totalContentHeight = fileSectionLayouts[fileSectionLayouts.length - 1]?.sectionBottom ?? 0; + // Read the live scroll box position during render so pinned-header ownership flips + // immediately after imperative scrolls instead of waiting for the polled viewport snapshot. + const effectiveScrollTop = scrollRef.current?.scrollTop ?? scrollViewport.top; + const pinnedHeaderFile = useMemo(() => { + if (files.length === 0) { + return null; + } + + // The current file header always owns the pinned top row. + // Use the previous visible row to decide ownership so the next file's real header can still + // scroll through the stream before the pinned header hands off to it on the following row. + const owner = findHeaderOwningFileSection( + fileSectionLayouts, + Math.max(0, effectiveScrollTop - 1), + ); + + return owner ? (files[owner.sectionIndex] ?? null) : (files[0] ?? null); + }, [effectiveScrollTop, fileSectionLayouts, files]); + const pinnedHeaderFileId = pinnedHeaderFile?.id ?? null; + + const copySelectionContext = useMemo( + (): CopySelectionContext => ({ + codeHorizontalOffset, + copyDecorations, + files, + fileSectionLayouts, + headerLabelWidth, + headerStatsWidth, + layout, + pinnedHeaderFile, + sectionGeometry, + showHunkHeaders, + showLineNumbers, + theme, + width: diffContentWidth, + wrapLines, + }), + [ + codeHorizontalOffset, + copyDecorations, + diffContentWidth, + fileSectionLayouts, + files, + headerLabelWidth, + headerStatsWidth, + layout, + pinnedHeaderFile, + sectionGeometry, + showHunkHeaders, + showLineNumbers, + theme, + wrapLines, + ], + ); + + // In split layout, anchor the visible selection (and clipboard copy) to whichever side of + // the diff the drag began on. Stack layout has only one column, so the side stays undefined. + const copySelectionSide: CopySelectionSide | undefined = useMemo(() => { + if (!copySelectionDrag || copySelectionDrag.anchor.kind !== "review-row") { + return undefined; + } + return resolveCopySelectionSide( + copySelectionDrag.anchor.column, + layout, + diffContentWidth, + ); + }, [copySelectionDrag, diffContentWidth, layout]); + + const copySelectedRowKeysByFile = useMemo( + () => + buildCopySelectedRowKeys({ + drag: copySelectionDrag, + fileSectionLayouts, + sectionGeometry, + width: diffContentWidth, + }), + [copySelectionDrag, diffContentWidth, fileSectionLayouts, sectionGeometry], + ); + + /** Copy selected text through the injected boundary or the renderer's OSC 52 clipboard support. */ + const copySelectionText = useCallback( + (text: string) => { + if (text.length === 0) { + return; + } + + if (onCopySelectionText) { + onCopySelectionText(text); + return; + } + + const supportsOsc52 = renderer.isOsc52Supported?.() ?? false; + if (supportsOsc52 && typeof renderer.copyToClipboardOSC52 === "function") { + renderer.copyToClipboardOSC52(text); + onCopyFeedback?.("Copied selection to clipboard"); + return; + } + + onCopyFeedback?.( + "Clipboard copy unsupported in this terminal (enable OSC 52 to capture selections)", + ); + }, + [onCopyFeedback, onCopySelectionText, renderer], + ); + + /** Convert one mouse event into a review-stream copy-selection point. */ + const resolveCopySelectionPoint = useCallback( + (event: TuiMouseEvent): CopySelectionPoint | null => { + const scrollBox = scrollRef.current; + if (!scrollBox) { + return null; + } + + const reviewPaneTopChromeRows = pagerMode ? 0 : 2; + const pinnedHeaderHeight = pinnedHeaderFileId ? 1 : 0; + const paneY = Math.floor(event.y - screenTop); + const pinnedHeaderY = reviewPaneTopChromeRows; + if (copyDecorations && pinnedHeaderFileId && paneY === pinnedHeaderY) { + return { + kind: "pinned-header", + column: clampCopyColumn(Math.floor(event.x - screenLeft), diffContentWidth), + fileId: pinnedHeaderFileId, + nextVisualRow: Math.floor(scrollBox.scrollTop ?? 0), + }; + } + + const paneChromeHeight = reviewPaneTopChromeRows + pinnedHeaderHeight; + const viewportY = Math.floor(event.y - screenTop - paneChromeHeight); + if (viewportY < 0 || viewportY >= Math.max(1, scrollBox.viewport.height ?? 0)) { + return null; + } + + return findCopySelectionPoint({ + column: Math.floor(event.x - screenLeft), + copyDecorations, + fileSectionLayouts, + sectionGeometry, + visualRow: Math.floor((scrollBox.scrollTop ?? 0) + viewportY), + width: diffContentWidth, + }); + }, + [ + copyDecorations, + diffContentWidth, + fileSectionLayouts, + pagerMode, + pinnedHeaderFileId, + screenLeft, + screenTop, + scrollRef, + sectionGeometry, + ], + ); + + // OpenTUI starts a native cross-renderable text selection on mouse-down over any selectable + // before our handler runs. That native selection ignores element bounds and paints + // across the whole screen, so we eagerly clear it whenever Hunk owns the drag. + const suppressNativeSelection = useCallback(() => { + if (renderer.hasSelection) { + renderer.clearSelection(); + } + }, [renderer]); + + /** Start selecting diff text when the user drags inside the review stream. */ + const beginCopySelection = useCallback( + (event: TuiMouseEvent) => { + if (event.button !== MouseButton.LEFT) { + return; + } + + const point = resolveCopySelectionPoint(event); + if (!point) { + setCopySelectionDrag(null); + return; + } + + // Detect double-click and triple-click for word/line selection. + const now = Date.now(); + const timeSinceLastClick = now - lastClickTimeRef.current; + lastClickTimeRef.current = now; + + let clickCount = 1; + if (timeSinceLastClick < 350 && timeSinceLastClick >= 0) { + clickCountRef.current += 1; + clickCount = Math.min(clickCountRef.current, 3); + } else { + clickCountRef.current = 1; + } + + if (clickCount >= 2 && point.kind === "review-row") { + const expanded = expandSelectionPoint( + point, + clickCount as 2 | 3, + copySelectionContext, + ); + if (expanded) { + setCopySelectionDrag({ + anchor: { ...point, column: expanded.startCol }, + focus: { ...point, column: expanded.endCol }, + moved: true, + }); + suppressNativeSelection(); + event.preventDefault(); + event.stopPropagation(); + return; + } + } + + setCopySelectionDrag({ anchor: point, focus: point, moved: false }); + suppressNativeSelection(); + event.preventDefault(); + event.stopPropagation(); + }, + [copySelectionContext, resolveCopySelectionPoint, suppressNativeSelection], + ); + + /** Extend the active diff text selection while the pointer moves. */ + const updateCopySelection = useCallback( + (event: TuiMouseEvent) => { + setCopySelectionDrag((current) => { + if (!current) { + return current; + } + + const point = resolveCopySelectionPoint(event); + if (!point) { + return current; + } + + return { + anchor: current.anchor, + focus: point, + moved: current.moved || !copySelectionPointsEqual(point, current.anchor), + }; + }); + + if (copySelectionDrag) { + suppressNativeSelection(); + event.preventDefault(); + event.stopPropagation(); + } + }, + [copySelectionDrag, resolveCopySelectionPoint, suppressNativeSelection], + ); + + /** Finish a drag selection and copy its rendered text. */ + const endCopySelection = useCallback( + (event?: TuiMouseEvent) => { + const current = copySelectionDrag; + if (!current) { + return; + } + + setCopySelectionDrag(null); + event?.preventDefault(); + event?.stopPropagation(); + + if (!current.moved) { + return; + } + + const { start, end } = normalizeCopySelectionRange(current.anchor, current.focus); + const text = renderCopySelectionText({ + context: copySelectionContext, + end, + side: copySelectionSide, + start, + }); + copySelectionText(text); + }, + [copySelectionDrag, copySelectionContext, copySelectionSide, copySelectionText], + ); + + // Expose the cancel hook so an ancestor (App's outer container) can release a stuck drag when + // the pointer leaves the diff pane and is released over the sidebar, menu bar, or status bar. + useEffect(() => { + if (!cancelCopySelectionRef) { + return; + } + cancelCopySelectionRef.current = () => endCopySelection(); + return () => { + if (cancelCopySelectionRef.current) { + cancelCopySelectionRef.current = null; + } + }; + }, [cancelCopySelectionRef, endCopySelection]); + /** Clamp one requested review scroll target against the latest planned content height. */ const clampReviewScrollTop = useCallback( (requestedTop: number, viewportHeight: number) => @@ -534,10 +854,6 @@ export function DiffPane({ } }, [files, highlightPrefetchFileIds, theme.appearance]); - // Read the live scroll box position during render so pinned-header ownership flips - // immediately after imperative scrolls instead of waiting for the polled viewport snapshot. - const effectiveScrollTop = scrollRef.current?.scrollTop ?? scrollViewport.top; - // Keep the selected file/hunk derived from the visible viewport for actual scroll-driven // movement, while leaving the initial mount and non-scroll relayouts alone. useLayoutEffect(() => { @@ -585,23 +901,6 @@ export function DiffPane({ selectedHunkIndex, ]); - const pinnedHeaderFile = useMemo(() => { - if (files.length === 0) { - return null; - } - - // The current file header always owns the pinned top row. - // Use the previous visible row to decide ownership so the next file's real header can still - // scroll through the stream before the pinned header hands off to it on the following row. - const owner = findHeaderOwningFileSection( - fileSectionLayouts, - Math.max(0, effectiveScrollTop - 1), - ); - - return owner ? (files[owner.sectionIndex] ?? null) : (files[0] ?? null); - }, [effectiveScrollTop, fileSectionLayouts, files]); - const pinnedHeaderFileId = pinnedHeaderFile?.id ?? null; - useLayoutEffect(() => { renderer.intermediateRender(); }, [renderer, pinnedHeaderFileId]); @@ -1099,12 +1398,20 @@ export function DiffPane({ paddingX: 0, flexDirection: "column", }} + onMouseDragEnd={endCopySelection} + onMouseUp={endCopySelection} > {files.length > 0 ? ( {/* Always pin the current file header in a dedicated top row. */} {pinnedHeaderFile ? ( - + ; selectedHunkIndex: number; + copySelectedRowRanges?: Map; + copySelectedSide?: "left" | "right"; shouldLoadHighlight: boolean; sectionGeometry?: DiffSectionGeometry; separatorWidth: number; @@ -40,6 +43,8 @@ function DiffSectionComponent({ headerStatsWidth, layout, selectedHunkIndex, + copySelectedRowRanges, + copySelectedSide, shouldLoadHighlight, sectionGeometry, separatorWidth, @@ -98,6 +103,8 @@ function DiffSectionComponent({ showHunkHeaders={showHunkHeaders} wrapLines={wrapLines} codeHorizontalOffset={codeHorizontalOffset} + copySelectedRowRanges={copySelectedRowRanges} + copySelectedSide={copySelectedSide} theme={theme} width={viewWidth} annotatedHunkIndices={annotatedHunkIndices} @@ -124,6 +131,8 @@ export const DiffSection = memo(DiffSectionComponent, (previous, next) => { previous.headerStatsWidth === next.headerStatsWidth && previous.layout === next.layout && previous.selectedHunkIndex === next.selectedHunkIndex && + previous.copySelectedRowRanges === next.copySelectedRowRanges && + previous.copySelectedSide === next.copySelectedSide && previous.shouldLoadHighlight === next.shouldLoadHighlight && previous.sectionGeometry === next.sectionGeometry && previous.separatorWidth === next.separatorWidth && diff --git a/src/ui/components/panes/copySelection.test.ts b/src/ui/components/panes/copySelection.test.ts new file mode 100644 index 00000000..5d39972a --- /dev/null +++ b/src/ui/components/panes/copySelection.test.ts @@ -0,0 +1,518 @@ +import { describe, expect, test } from "bun:test"; +import { parseDiffFromFile } from "@pierre/diffs"; +import type { DiffFile } from "../../../core/types"; +import { resolveTheme } from "../../themes"; +import { measureDiffSectionGeometry } from "../../lib/diffSectionGeometry"; +import { buildFileSectionLayouts } from "../../lib/fileSectionLayout"; +import { + buildCopySelectedRowKeys, + clampCopyColumn, + copySelectionPointsEqual, + expandSelectionPoint, + findCopySelectionPoint, + normalizeCopySelectionRange, + renderCopySelectionText, + resolveCopySelectionSide, + type CopySelectionContext, + type CopySelectionDrag, + type CopySelectionPoint, +} from "./copySelection"; +import { + DIFF_RAIL_PREFIX_WIDTH, + resolveSplitCellGeometry, + resolveSplitPaneWidths, +} from "../../diff/codeColumns"; + +function createDiffFile(): DiffFile { + const metadata = parseDiffFromFile( + { + name: "example.ts", + contents: "export const answer = 41;\nexport const stable = true;\n", + cacheKey: "before", + }, + { + name: "example.ts", + contents: + "export const answer = 42;\nexport const stable = true;\nexport const added = true;\n", + cacheKey: "after", + }, + { context: 3 }, + true, + ); + + return { + id: "example", + path: "example.ts", + patch: "", + language: "typescript", + stats: { + additions: 2, + deletions: 1, + }, + metadata, + agent: null, + }; +} + +function buildContext( + layout: "stack" | "split" = "stack", + width = 120, +): { + context: CopySelectionContext; + fileSectionLayouts: ReturnType; + sectionGeometry: ReturnType[]; +} { + const theme = resolveTheme("midnight", null); + const file = createDiffFile(); + const geometry = measureDiffSectionGeometry(file, layout, true, theme, [], width, true, false); + const sectionGeometry = [geometry]; + const fileSectionLayouts = buildFileSectionLayouts([file], [geometry.bodyHeight]); + + const context: CopySelectionContext = { + codeHorizontalOffset: 0, + copyDecorations: true, + files: [file], + fileSectionLayouts, + headerLabelWidth: 60, + headerStatsWidth: 12, + layout, + pinnedHeaderFile: file, + sectionGeometry, + showHunkHeaders: true, + showLineNumbers: true, + theme, + width, + wrapLines: false, + }; + + return { context, fileSectionLayouts, sectionGeometry }; +} + +describe("clampCopyColumn", () => { + test("clamps below zero to zero", () => { + expect(clampCopyColumn(-5, 10)).toBe(0); + }); + + test("clamps above the rendered width", () => { + expect(clampCopyColumn(99, 10)).toBe(9); + }); + + test("returns zero when width is zero", () => { + expect(clampCopyColumn(5, 0)).toBe(0); + }); +}); + +describe("copySelectionPointsEqual", () => { + test("rejects different kinds even at the same column", () => { + const a: CopySelectionPoint = { kind: "review-row", column: 1, visualRow: 1 }; + const b: CopySelectionPoint = { + kind: "pinned-header", + column: 1, + fileId: "example", + nextVisualRow: 1, + }; + expect(copySelectionPointsEqual(a, b)).toBe(false); + }); + + test("matches identical review-row points", () => { + const a: CopySelectionPoint = { kind: "review-row", column: 2, visualRow: 4 }; + const b: CopySelectionPoint = { kind: "review-row", column: 2, visualRow: 4 }; + expect(copySelectionPointsEqual(a, b)).toBe(true); + }); + + test("treats pinned-header points with different file ids as distinct", () => { + const a: CopySelectionPoint = { + kind: "pinned-header", + column: 0, + fileId: "one", + nextVisualRow: 0, + }; + const b: CopySelectionPoint = { + kind: "pinned-header", + column: 0, + fileId: "two", + nextVisualRow: 0, + }; + expect(copySelectionPointsEqual(a, b)).toBe(false); + }); +}); + +describe("normalizeCopySelectionRange", () => { + test("orders forward selections by row then column", () => { + const anchor: CopySelectionPoint = { kind: "review-row", column: 2, visualRow: 1 }; + const focus: CopySelectionPoint = { kind: "review-row", column: 8, visualRow: 1 }; + const { start, end } = normalizeCopySelectionRange(anchor, focus); + expect(start).toBe(anchor); + expect(end).toBe(focus); + }); + + test("flips reverse selections so start <= end", () => { + const anchor: CopySelectionPoint = { kind: "review-row", column: 5, visualRow: 3 }; + const focus: CopySelectionPoint = { kind: "review-row", column: 2, visualRow: 1 }; + const { start, end } = normalizeCopySelectionRange(anchor, focus); + expect(start).toBe(focus); + expect(end).toBe(anchor); + }); + + test("sorts a pinned-header point above its body", () => { + const header: CopySelectionPoint = { + kind: "pinned-header", + column: 0, + fileId: "example", + nextVisualRow: 2, + }; + const body: CopySelectionPoint = { kind: "review-row", column: 0, visualRow: 2 }; + const { start, end } = normalizeCopySelectionRange(body, header); + expect(start).toBe(header); + expect(end).toBe(body); + }); +}); + +describe("findCopySelectionPoint", () => { + test("returns a review-row point for a row inside the body", () => { + const { context, fileSectionLayouts, sectionGeometry } = buildContext(); + const probeRow = fileSectionLayouts[0]!.bodyTop; + const point = findCopySelectionPoint({ + column: 4, + copyDecorations: true, + fileSectionLayouts, + sectionGeometry, + visualRow: probeRow, + width: context.width, + }); + + expect(point).not.toBeNull(); + expect(point?.kind).toBe("review-row"); + expect(point?.visualRow).toBe(probeRow); + expect(point?.column).toBe(4); + }); + + test("returns null for rows past the end of the stream", () => { + const { context, fileSectionLayouts, sectionGeometry } = buildContext(); + const lastLayout = fileSectionLayouts[fileSectionLayouts.length - 1]!; + + const point = findCopySelectionPoint({ + column: 0, + copyDecorations: true, + fileSectionLayouts, + sectionGeometry, + visualRow: lastLayout.sectionBottom + 50, + width: context.width, + }); + + expect(point).toBeNull(); + }); +}); + +describe("renderCopySelectionText", () => { + test("produces decorated text for a single-row drag", () => { + const { context, fileSectionLayouts } = buildContext(); + const start: CopySelectionPoint = { + kind: "review-row", + column: 0, + visualRow: fileSectionLayouts[0]!.bodyTop, + }; + const end: CopySelectionPoint = { + kind: "review-row", + column: context.width - 1, + visualRow: fileSectionLayouts[0]!.bodyTop, + }; + + const text = renderCopySelectionText({ context, start, end }); + expect(text.length).toBeGreaterThan(0); + // Decorated output keeps the diff rail marker at the row prefix. + expect(text.startsWith("▌")).toBe(true); + }); + + test("strips all decorations when copyDecorations is disabled", () => { + const { context, fileSectionLayouts } = buildContext(); + const undecoratedContext: CopySelectionContext = { ...context, copyDecorations: false }; + + const start: CopySelectionPoint = { + kind: "review-row", + column: 0, + visualRow: fileSectionLayouts[0]!.bodyTop, + }; + const end: CopySelectionPoint = { + kind: "review-row", + column: undecoratedContext.width - 1, + visualRow: fileSectionLayouts[0]!.sectionBottom - 1, + }; + + const text = renderCopySelectionText({ context: undecoratedContext, start, end }); + expect(text).not.toContain("▌"); + expect(text).toContain("export const answer = 41;"); + expect(text).toContain("export const answer = 42;"); + }); + + test("includes the pinned header when the drag starts in it", () => { + const { context, fileSectionLayouts } = buildContext(); + const start: CopySelectionPoint = { + kind: "pinned-header", + column: 0, + fileId: "example", + nextVisualRow: fileSectionLayouts[0]!.bodyTop, + }; + const end: CopySelectionPoint = { + kind: "review-row", + column: context.width - 1, + visualRow: fileSectionLayouts[0]!.bodyTop, + }; + + const text = renderCopySelectionText({ context, start, end }); + expect(text).toContain("example.ts"); + }); +}); + +describe("resolveCopySelectionSide", () => { + test("returns undefined in stack layout", () => { + expect(resolveCopySelectionSide(10, "stack", 120)).toBeUndefined(); + expect(resolveCopySelectionSide(80, "stack", 120)).toBeUndefined(); + }); + + test("returns 'left' for columns inside the split left pane", () => { + expect(resolveCopySelectionSide(0, "split", 120)).toBe("left"); + expect(resolveCopySelectionSide(10, "split", 120)).toBe("left"); + }); + + test("returns 'right' for columns at or past the split midpoint", () => { + expect(resolveCopySelectionSide(100, "split", 120)).toBe("right"); + }); +}); + +describe("renderCopySelectionText with side", () => { + test("includes only the left side text when side is 'left' and decorations are off", () => { + const { context, fileSectionLayouts } = buildContext("split"); + const splitContext: CopySelectionContext = { + ...context, + copyDecorations: false, + }; + const start: CopySelectionPoint = { + kind: "review-row", + column: 0, + visualRow: fileSectionLayouts[0]!.bodyTop, + }; + const end: CopySelectionPoint = { + kind: "review-row", + column: 10, + visualRow: fileSectionLayouts[0]!.sectionBottom - 1, + }; + + const text = renderCopySelectionText({ + context: splitContext, + start, + end, + side: "left", + }); + expect(text).toContain("export const answer = 41;"); + expect(text).not.toContain("export const answer = 42;"); + }); + + test("includes only the right side text when side is 'right' and decorations are off", () => { + const { context, fileSectionLayouts } = buildContext("split"); + const splitContext: CopySelectionContext = { + ...context, + copyDecorations: false, + }; + const start: CopySelectionPoint = { + kind: "review-row", + column: 0, + visualRow: fileSectionLayouts[0]!.bodyTop, + }; + const end: CopySelectionPoint = { + kind: "review-row", + column: 10, + visualRow: fileSectionLayouts[0]!.sectionBottom - 1, + }; + + const text = renderCopySelectionText({ + context: splitContext, + start, + end, + side: "right", + }); + expect(text).toContain("export const answer = 42;"); + expect(text).not.toContain("export const answer = 41;"); + }); +}); + +describe("buildCopySelectedRowKeys", () => { + test("returns an empty map when the drag has not moved", () => { + const { fileSectionLayouts, sectionGeometry } = buildContext(); + const point: CopySelectionPoint = { kind: "review-row", column: 0, visualRow: 0 }; + const drag: CopySelectionDrag = { anchor: point, focus: point, moved: false }; + + expect(buildCopySelectedRowKeys({ drag, fileSectionLayouts, sectionGeometry, width: 120 }).size).toBe(0); + }); + + test("collects every row key intersected by a multi-row drag", () => { + const { fileSectionLayouts, sectionGeometry } = buildContext(); + const firstLayout = fileSectionLayouts[0]!; + const anchor: CopySelectionPoint = { + kind: "review-row", + column: 0, + visualRow: firstLayout.bodyTop, + }; + const focus: CopySelectionPoint = { + kind: "review-row", + column: 0, + visualRow: firstLayout.sectionBottom - 1, + }; + + const map = buildCopySelectedRowKeys({ + drag: { anchor, focus, moved: true }, + fileSectionLayouts, + sectionGeometry, + width: 120, + }); + + const rows = map.get("example"); + expect(rows).toBeDefined(); + expect(rows?.size ?? 0).toBeGreaterThan(0); + }); +}); + +describe("expandSelectionPoint", () => { + test("triple-click in stack selects the full width", () => { + const { context, fileSectionLayouts } = buildContext("stack"); + const section = fileSectionLayouts[0]!; + const point: CopySelectionPoint = { + kind: "review-row", + column: 40, + visualRow: section.bodyTop, + }; + const result = expandSelectionPoint(point, 3, context); + expect(result).toEqual({ startCol: 0, endCol: context.width - 1 }); + }); + + test("triple-click in split on left side stays within left pane", () => { + const { context, fileSectionLayouts } = buildContext("split"); + const { leftWidth } = resolveSplitPaneWidths(context.width); + const section = fileSectionLayouts[0]!; + // Column clearly inside the left pane + const point: CopySelectionPoint = { + kind: "review-row", + column: 5, + visualRow: section.bodyTop, + }; + const result = expandSelectionPoint(point, 3, context); + expect(result).not.toBeNull(); + if (result) { + // Left side: columns 0..leftWidth-1 + expect(result.startCol).toBe(0); + expect(result.endCol).toBe(leftWidth - 1); + + // The anchor/focus side must remain "left" + const side = resolveCopySelectionSide(result.startCol, "split", context.width); + expect(side).toBe("left"); + } + }); + + test("triple-click in split on right side stays within right pane", () => { + const { context, fileSectionLayouts } = buildContext("split"); + const { leftWidth } = resolveSplitPaneWidths(context.width); + const section = fileSectionLayouts[0]!; + // Column clearly inside the right pane + const point: CopySelectionPoint = { + kind: "review-row", + column: leftWidth + DIFF_RAIL_PREFIX_WIDTH + 1, + visualRow: section.bodyTop, + }; + const result = expandSelectionPoint(point, 3, context); + expect(result).not.toBeNull(); + if (result) { + // Right side: columns leftWidth..width-1 + expect(result.startCol).toBe(leftWidth); + expect(result.endCol).toBe(context.width - 1); + + // The anchor/focus side must remain "right" + const side = resolveCopySelectionSide(result.startCol, "split", context.width); + expect(side).toBe("right"); + } + }); +}); + +describe("renderCopySelectionText in split with side", () => { + test("B side text with copyDecorations=true uses correct column offsets", () => { + const { context, fileSectionLayouts } = buildContext("split"); + const section = fileSectionLayouts[0]!; + const { leftWidth } = resolveSplitPaneWidths(context.width); + + // B (right) side first body row, column inside the right pane + const start: CopySelectionPoint = { + kind: "review-row", + column: leftWidth + DIFF_RAIL_PREFIX_WIDTH + 1, + visualRow: section.bodyTop, + }; + const end: CopySelectionPoint = { + kind: "review-row", + column: leftWidth + DIFF_RAIL_PREFIX_WIDTH + 1, + visualRow: section.sectionBottom - 1, + }; + + // With decorations enabled and side="right", the text must be non-empty + // and should contain B-side content ("export const answer = 42") + const text = renderCopySelectionText({ + context, + start, + end, + side: "right", + }); + expect(text.length).toBeGreaterThan(0); + expect(text).toContain("export const answer = 42;"); + }); + + test("A side text with copyDecorations=true stays intact", () => { + const { context, fileSectionLayouts } = buildContext("split"); + const section = fileSectionLayouts[0]!; + + const start: CopySelectionPoint = { + kind: "review-row", + column: DIFF_RAIL_PREFIX_WIDTH + 1, + visualRow: section.bodyTop, + }; + const end: CopySelectionPoint = { + kind: "review-row", + column: DIFF_RAIL_PREFIX_WIDTH + 1, + visualRow: section.sectionBottom - 1, + }; + + const text = renderCopySelectionText({ + context, + start, + end, + side: "left", + }); + expect(text.length).toBeGreaterThan(0); + expect(text).toContain("export const answer = 41;"); + expect(text).not.toContain("export const answer = 42;"); + }); + + test("decorated B side multi-line selection includes all lines", () => { + const { context, fileSectionLayouts } = buildContext("split"); + const section = fileSectionLayouts[0]!; + const { leftWidth } = resolveSplitPaneWidths(context.width); + + // B side: select first row to last row + const start: CopySelectionPoint = { + kind: "review-row", + column: leftWidth + DIFF_RAIL_PREFIX_WIDTH + 1, + visualRow: section.bodyTop, + }; + const end: CopySelectionPoint = { + kind: "review-row", + column: context.width - 1, + visualRow: section.sectionBottom - 1, + }; + + const text = renderCopySelectionText({ + context, + start, + end, + side: "right", + }); + expect(text.length).toBeGreaterThan(0); + // First line should be included + expect(text).toContain("export const answer = 42;"); + }); +}); diff --git a/src/ui/components/panes/copySelection.ts b/src/ui/components/panes/copySelection.ts new file mode 100644 index 00000000..a0c51f13 --- /dev/null +++ b/src/ui/components/panes/copySelection.ts @@ -0,0 +1,563 @@ +import type { DiffFile, LayoutMode } from "../../../core/types"; +import { + DIFF_RAIL_PREFIX_WIDTH, + resolveSplitCellGeometry, + resolveSplitPaneWidths, + resolveStackCellGeometry, +} from "../../diff/codeColumns"; +import { renderCodeOnlyPlannedRowText, renderDecoratedPlannedRowText } from "../../diff/renderRows"; +import { type DiffSectionGeometry, type DiffSectionRowBounds } from "../../lib/diffSectionGeometry"; +import type { FileSectionLayout } from "../../lib/fileSectionLayout"; +import { fileLabelParts } from "../../lib/files"; +import { fitText } from "../../lib/text"; +import type { AppTheme } from "../../themes"; + +export type CopySelectionPoint = + | { + kind: "review-row"; + column: number; + visualRow: number; + } + | { + kind: "pinned-header"; + column: number; + fileId: string; + nextVisualRow: number; + }; + +// In split layout the drag is anchored to one side of the diff (left = old / A, right = new / B) +// based on the anchor column. In stack layout there is only one column, so side is undefined. +export type CopySelectionSide = "left" | "right"; + +export interface CopySelectionDrag { + anchor: CopySelectionPoint; + focus: CopySelectionPoint; + moved: boolean; +} + +export interface CopySelectionContext { + codeHorizontalOffset: number; + copyDecorations: boolean; + files: DiffFile[]; + fileSectionLayouts: FileSectionLayout[]; + headerLabelWidth: number; + headerStatsWidth: number; + layout: Exclude; + pinnedHeaderFile?: DiffFile | null; + sectionGeometry: DiffSectionGeometry[]; + showHunkHeaders: boolean; + showLineNumbers: boolean; + theme: AppTheme; + width: number; + wrapLines: boolean; +} + +/** Resolve which split side a column belongs to in split layout. */ +export function resolveCopySelectionSide( + column: number, + layout: Exclude, + width: number, +): CopySelectionSide | undefined { + if (layout !== "split") { + return undefined; + } + const { leftWidth } = resolveSplitPaneWidths(width); + return column < leftWidth ? "left" : "right"; +} + +/** Clamp one terminal column into the rendered diff body. */ +export function clampCopyColumn(column: number, width: number) { + return Math.min(Math.max(0, column), Math.max(0, width - 1)); +} + +/** Return whether one row bounds entry owns the requested file-local visual row. */ +function rowBoundsContainsVisualRow(bounds: DiffSectionRowBounds, visualRow: number) { + return bounds.height > 0 && visualRow >= bounds.top && visualRow < bounds.top + bounds.height; +} + +// Pinned-header points sort to (nextVisualRow - 1) so they slot right above the first visible +// body row, matching what the user sees at the top of the pane. +function copySelectionSortRow(point: CopySelectionPoint) { + return point.kind === "pinned-header" ? point.nextVisualRow - 1 : point.visualRow; +} + +/** Return the selected body row range, excluding any standalone pinned header row. */ +function copySelectionBodyRange(start: CopySelectionPoint, end: CopySelectionPoint) { + const startRow = start.kind === "pinned-header" ? start.nextVisualRow : start.visualRow; + const endRow = end.kind === "pinned-header" ? end.nextVisualRow - 1 : end.visualRow; + + return { startRow, endRow }; +} + +/** Return whether two points represent the same selectable terminal cell. */ +export function copySelectionPointsEqual(left: CopySelectionPoint, right: CopySelectionPoint) { + if (left.kind !== right.kind || left.column !== right.column) { + return false; + } + + if (left.kind === "pinned-header" && right.kind === "pinned-header") { + return left.fileId === right.fileId && left.nextVisualRow === right.nextVisualRow; + } + + return ( + left.kind === "review-row" && right.kind === "review-row" && left.visualRow === right.visualRow + ); +} + +/** Order two selection points by terminal row first, then column. */ +export function normalizeCopySelectionRange(anchor: CopySelectionPoint, focus: CopySelectionPoint) { + const anchorRow = copySelectionSortRow(anchor); + const focusRow = copySelectionSortRow(focus); + + if (anchorRow < focusRow || (anchorRow === focusRow && anchor.column <= focus.column)) { + return { start: anchor, end: focus }; + } + + return { start: focus, end: anchor }; +} + +/** Trim padding introduced only to fill fixed terminal cells. */ +function trimCopiedLine(line: string) { + return line.replace(/[ \t]+$/g, ""); +} + +/** Render one file header as plain text using the same visible columns as DiffFileHeaderRow. */ +function renderFileHeaderCopyText({ + file, + headerLabelWidth, + headerStatsWidth, + width, +}: { + file: DiffFile; + headerLabelWidth: number; + headerStatsWidth: number; + width: number; +}) { + const additionsText = `+${file.stats.additions}${file.statsTruncated ? "+" : ""}`; + const deletionsText = `-${file.stats.deletions}`; + const statsText = `${additionsText} ${deletionsText}`.padStart(headerStatsWidth); + const { filename, stateLabel } = fileLabelParts(file); + const label = `${fitText( + filename, + Math.max(1, headerLabelWidth - (stateLabel?.length ?? 0)), + )}${stateLabel ?? ""}`; + const availableGap = Math.max(1, width - 2 - label.length - statsText.length); + + return ` ${label}${" ".repeat(availableGap)}${statsText} `.slice(0, width).padEnd(width); +} + +// The "pinned-header" point variant is constructed inline by callers that observe a click on the +// pinned-header row directly. This function only resolves coordinates against the scrolling review +// body, so it always returns a "review-row" point (or null when the row is outside the stream). +export function findCopySelectionPoint({ + column, + copyDecorations, + fileSectionLayouts, + sectionGeometry, + visualRow, + width, +}: { + column: number; + copyDecorations: boolean; + fileSectionLayouts: FileSectionLayout[]; + sectionGeometry: DiffSectionGeometry[]; + visualRow: number; + width: number; +}): Extract | null { + for (const section of fileSectionLayouts) { + if ( + copyDecorations && + section.headerTop < section.bodyTop && + visualRow >= section.headerTop && + visualRow < section.bodyTop + ) { + return { + kind: "review-row", + column: clampCopyColumn(column, width), + visualRow, + }; + } + + if (visualRow < section.bodyTop || visualRow >= section.bodyTop + section.bodyHeight) { + continue; + } + + const geometry = sectionGeometry[section.sectionIndex]; + if (!geometry) { + return null; + } + + const bodyRow = visualRow - section.bodyTop; + const rowIndex = geometry.rowBounds.findIndex((bounds) => + rowBoundsContainsVisualRow(bounds, bodyRow), + ); + if (rowIndex < 0) { + return null; + } + + return { + kind: "review-row", + column: clampCopyColumn(column, width), + visualRow, + }; + } + + return null; +} + +/** Render the selected planned rows into clipboard text. */ +export function renderCopySelectionText({ + context, + end, + side, + start, +}: { + context: CopySelectionContext; + end: CopySelectionPoint; + side?: CopySelectionSide; + start: CopySelectionPoint; +}) { + const lines: string[] = []; + const { + codeHorizontalOffset, + copyDecorations, + files, + fileSectionLayouts, + headerLabelWidth, + headerStatsWidth, + pinnedHeaderFile, + sectionGeometry, + showHunkHeaders, + showLineNumbers, + theme, + width, + wrapLines, + } = context; + + if ( + copyDecorations && + pinnedHeaderFile && + start.kind === "pinned-header" && + start.fileId === pinnedHeaderFile.id + ) { + const line = renderFileHeaderCopyText({ + file: pinnedHeaderFile, + headerLabelWidth, + headerStatsWidth, + width, + }); + const endColumn = + end.kind === "pinned-header" && end.fileId === start.fileId + ? end.column + : Math.max(0, line.length - 1); + lines.push(trimCopiedLine(line.slice(start.column, endColumn + 1))); + } + + const { startRow, endRow } = copySelectionBodyRange(start, end); + + for (const section of fileSectionLayouts) { + if (section.sectionBottom <= startRow || section.headerTop > endRow) { + continue; + } + + if ( + copyDecorations && + section.headerTop < section.bodyTop && + section.headerTop >= startRow && + section.headerTop <= endRow + ) { + const file = files[section.sectionIndex]; + if (file) { + const line = renderFileHeaderCopyText({ + file, + headerLabelWidth, + headerStatsWidth, + width, + }); + const startColumn = + start.kind === "review-row" && section.headerTop === start.visualRow ? start.column : 0; + const endColumn = + end.kind === "review-row" && section.headerTop === end.visualRow + ? end.column + : Math.max(0, line.length - 1); + lines.push(trimCopiedLine(line.slice(startColumn, endColumn + 1))); + } + } + + const geometry = sectionGeometry[section.sectionIndex]; + if (!geometry) { + continue; + } + + for (let rowIndex = 0; rowIndex < geometry.rowBounds.length; rowIndex += 1) { + const rowBounds = geometry.rowBounds[rowIndex]!; + const row = geometry.plannedRows[rowIndex]; + if (!row || rowBounds.height <= 0) { + continue; + } + + const rowTop = section.bodyTop + rowBounds.top; + const rowBottom = rowTop + rowBounds.height; + if (rowBottom <= startRow || rowTop > endRow) { + continue; + } + + const renderRowText = copyDecorations + ? renderDecoratedPlannedRowText + : renderCodeOnlyPlannedRowText; + const renderedLines = renderRowText(row, { + codeHorizontalOffset, + lineNumberDigits: geometry.lineNumberDigits, + showHunkHeaders, + showLineNumbers, + side, + theme, + width, + wrapLines, + }); + + if (!copyDecorations) { + lines.push(...renderedLines.map(trimCopiedLine).filter(Boolean)); + continue; + } + + // In split layout, `side` selects which pane text to render via + // renderDecoratedPlannedRowText. The returned lines start at the pane boundary, + // not at global column 0. Global column values from the selection points must be + // adjusted by subtracting the left-pane offset when side="right" so the slice + // aligns with the actual rendered string. + const paneOffset = + side === "right" && context.layout === "split" + ? resolveSplitPaneWidths(context.width).leftWidth + : 0; + + for (let lineIndex = 0; lineIndex < renderedLines.length; lineIndex += 1) { + const lineVisualRow = rowTop + lineIndex; + if (lineVisualRow < startRow || lineVisualRow > endRow) { + continue; + } + + const line = renderedLines[lineIndex] ?? ""; + const startColumn = + start.kind === "review-row" && lineVisualRow === start.visualRow + ? Math.max(0, start.column - paneOffset) + : 0; + const endColumn = + end.kind === "review-row" && lineVisualRow === end.visualRow + ? Math.min( + Math.max(0, line.length - 1), + Math.max(0, end.column - paneOffset), + ) + : Math.max(0, line.length - 1); + lines.push(trimCopiedLine(line.slice(startColumn, endColumn + 1))); + } + } + } + + return lines.join("\n").replace(/\n+$/g, ""); +} + +export interface CopySelectedRowRange { + /** Global column where the selection starts on this row. */ + startCol: number; + /** Global column where the selection ends on this row (inclusive). */ + endCol: number; +} + +/** + * Expand a single selection point to word or line boundaries for double/triple-click support. + * + * Returns the expanded column range (inclusive, global review-stream columns), or `null` if + * the row text cannot be resolved. + */ +export function expandSelectionPoint( + point: Extract, + clickCount: 2 | 3, + context: CopySelectionContext, +): { startCol: number; endCol: number } | null { + const { fileSectionLayouts, layout, sectionGeometry, showLineNumbers, width } = context; + + // Find the section and row at this visual row. + for (const section of fileSectionLayouts) { + if ( + point.visualRow < section.bodyTop || + point.visualRow >= section.bodyTop + section.bodyHeight + ) { + continue; + } + + const geometry = sectionGeometry[section.sectionIndex]; + if (!geometry) { + return null; + } + + const bodyRow = point.visualRow - section.bodyTop; + const rowIndex = geometry.rowBounds.findIndex((bounds) => + rowBoundsContainsVisualRow(bounds, bodyRow), + ); + if (rowIndex < 0) { + return null; + } + + const row = geometry.plannedRows[rowIndex]; + if (!row) { + return null; + } + + if (clickCount === 3) { + // Triple-click: select the entire rendered line. + // In split layout, scope to the side containing the click so triple-click never + // selects across both panes or resolves to the wrong side for copy/highlight. + if (layout === "split") { + const { leftWidth } = resolveSplitPaneWidths(width); + const clickSide = resolveCopySelectionSide(point.column, layout, width); + if (clickSide === "right") { + return { startCol: leftWidth, endCol: width - 1 }; + } + return { startCol: 0, endCol: Math.max(0, leftWidth - 1) }; + } + return { startCol: 0, endCol: width - 1 }; + } + + // Double-click: expand to word boundaries within the code content (excluding rail/gutter). + const side = resolveCopySelectionSide(point.column, layout, width); + + // Compute how many global columns the prefix and gutter consume so we can convert between + // code-local and global column spaces. + let globalContentStart: number; + if (layout === "split") { + const { leftWidth } = resolveSplitPaneWidths(width); + const paneOffset = side === "left" ? 0 : leftWidth; + const paneWidth = side === "left" ? leftWidth : width - leftWidth; + const { gutterWidth } = resolveSplitCellGeometry( + paneWidth, + geometry.lineNumberDigits, + showLineNumbers, + DIFF_RAIL_PREFIX_WIDTH, + ); + globalContentStart = paneOffset + DIFF_RAIL_PREFIX_WIDTH + gutterWidth; + } else { + const { gutterWidth } = resolveStackCellGeometry( + width, + geometry.lineNumberDigits, + showLineNumbers, + DIFF_RAIL_PREFIX_WIDTH, + ); + globalContentStart = DIFF_RAIL_PREFIX_WIDTH + gutterWidth; + } + + const lineIndex = bodyRow - geometry.rowBounds[rowIndex]!.top; + + // Use code-only text so word detection ignores the rail, line numbers, and diff signs. + const codeText = renderCodeOnlyPlannedRowText(row, { + codeHorizontalOffset: context.codeHorizontalOffset, + lineNumberDigits: geometry.lineNumberDigits, + showHunkHeaders: context.showHunkHeaders, + showLineNumbers, + side, + theme: context.theme, + width, + wrapLines: context.wrapLines, + }); + + const lineText = codeText[lineIndex]; + if (lineText === undefined || lineText.length === 0) { + return null; + } + + // Convert the global click column to a code-local column. + const localCol = Math.max(0, Math.min(lineText.length - 1, point.column - globalContentStart)); + let wordStart = localCol; + let wordEnd = localCol; + + // Expand left to word start. + while (wordStart > 0 && lineText[wordStart - 1] !== " " && lineText[wordStart - 1] !== "\t") { + wordStart -= 1; + } + // Expand right to word end (exclusive). + while (wordEnd < lineText.length && lineText[wordEnd] !== " " && lineText[wordEnd] !== "\t") { + wordEnd += 1; + } + + // Convert back to global columns. wordEnd is exclusive (one past last char), + // so endCol = wordEnd - 1 is inclusive. + return { + startCol: wordStart + globalContentStart, + endCol: wordEnd - 1 + globalContentStart, + }; + } + + return null; +} + +/** Build file-local row key ranges for the visible copy-selection highlight. */ +export function buildCopySelectedRowKeys({ + drag, + fileSectionLayouts, + sectionGeometry, + width, +}: { + drag: CopySelectionDrag | null; + fileSectionLayouts: FileSectionLayout[]; + sectionGeometry: DiffSectionGeometry[]; + /** Diff content width, used as the full-width range value for middle rows. */ + width: number; +}) { + const selected = new Map>(); + if (!drag?.moved) { + return selected; + } + + const { start, end } = normalizeCopySelectionRange(drag.anchor, drag.focus); + const { startRow, endRow } = copySelectionBodyRange(start, end); + for (const section of fileSectionLayouts) { + if (section.bodyTop + section.bodyHeight <= startRow || section.bodyTop > endRow) { + continue; + } + + const geometry = sectionGeometry[section.sectionIndex]; + if (!geometry) { + continue; + } + + for (const rowBounds of geometry.rowBounds) { + const rowTop = section.bodyTop + rowBounds.top; + const rowBottom = rowTop + rowBounds.height; + if (rowBounds.height <= 0 || rowBottom <= startRow || rowTop > endRow) { + continue; + } + + // Determine the global column range for this planned row. + // For unwrapped rows (height=1) this is straightforward. + // For wrapped rows (height>1) the same row key spans multiple visual rows; + // we use the visual row that overlaps the selection boundary to decide. + const rowLastVisualRow = rowBottom - 1; + let rangeStartCol: number; + let rangeEndCol: number; + + if (rowTop >= startRow && rowLastVisualRow <= endRow) { + // Row is fully inside the selection range. + rangeStartCol = rowTop === startRow ? start.column : 0; + rangeEndCol = rowLastVisualRow === endRow ? end.column : width - 1; + } else if (rowTop <= startRow && rowLastVisualRow >= startRow && rowLastVisualRow <= endRow) { + // Row starts above the selection and the last visual row is within it. + rangeStartCol = start.column; + rangeEndCol = rowLastVisualRow === endRow ? end.column : width - 1; + } else if (rowTop >= startRow && rowTop <= endRow && rowLastVisualRow >= endRow) { + // Row starts within the selection and extends past it. + rangeStartCol = rowTop === startRow ? start.column : 0; + rangeEndCol = end.column; + } else { + // Row spans across the entire selection (starts above, ends below). + rangeStartCol = start.column; + rangeEndCol = end.column; + } + + const fileRows = selected.get(section.fileId) ?? new Map(); + fileRows.set(rowBounds.key, { startCol: rangeStartCol, endCol: rangeEndCol }); + selected.set(section.fileId, fileRows); + } + } + + return selected; +} diff --git a/src/ui/diff/PierreDiffView.tsx b/src/ui/diff/PierreDiffView.tsx index 0060525d..4ded0485 100644 --- a/src/ui/diff/PierreDiffView.tsx +++ b/src/ui/diff/PierreDiffView.tsx @@ -2,6 +2,7 @@ import { useMemo } from "react"; import type { DiffFile, LayoutMode } from "../../core/types"; import { AgentInlineNote, AgentInlineNoteGuideCap } from "../components/panes/AgentInlineNote"; import type { VisibleAgentNote } from "../lib/agentAnnotations"; +import type { CopySelectedRowRange } from "../components/panes/copySelection"; import type { DiffSectionGeometry } from "../lib/diffSectionGeometry"; import { reviewRowId } from "../lib/ids"; import type { AppTheme } from "../themes"; @@ -20,6 +21,8 @@ const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; export function PierreDiffView({ annotatedHunkIndices = EMPTY_ANNOTATED_HUNK_INDICES, codeHorizontalOffset = 0, + copySelectedRowRanges, + copySelectedSide, file, layout, onOpenAgentNotesAtHunk, @@ -37,6 +40,8 @@ export function PierreDiffView({ }: { annotatedHunkIndices?: Set; codeHorizontalOffset?: number; + copySelectedRowRanges?: Map; + copySelectedSide?: "left" | "right"; file: DiffFile | undefined; layout: Exclude; onOpenAgentNotesAtHunk?: (hunkIndex: number) => void; @@ -189,6 +194,8 @@ export function PierreDiffView({ codeHorizontalOffset={codeHorizontalOffset} theme={theme} selected={plannedRow.row.hunkIndex === selectedHunkIndex} + copySelectedRowRange={copySelectedRowRanges?.get(plannedRow.key)} + copySelectedSide={copySelectedSide} annotated={ plannedRow.row.type === "hunk-header" && annotatedHunkIndices.has(plannedRow.row.hunkIndex) diff --git a/src/ui/diff/pierre.test.ts b/src/ui/diff/pierre.test.ts index cb64908f..356d3708 100644 --- a/src/ui/diff/pierre.test.ts +++ b/src/ui/diff/pierre.test.ts @@ -2,6 +2,8 @@ import { describe, expect, test } from "bun:test"; import { parseDiffFromFile } from "@pierre/diffs"; import type { DiffFile } from "../../core/types"; import { buildSplitRows, buildStackRows, loadHighlightedDiff, type DiffRow } from "./pierre"; +import { renderCodeOnlyPlannedRowText, renderDecoratedPlannedRowText } from "./renderRows"; +import { buildReviewRenderPlan } from "./reviewRenderPlan"; import { resolveTheme } from "../themes"; function createDiffFile(): DiffFile { @@ -164,6 +166,119 @@ describe("Pierre diff rows", () => { expect(additionRow.cell.newLineNumber).toBe(1); }); + test("renders planned split rows to copyable visible text", () => { + const file = createDiffFile(); + const theme = resolveTheme("midnight", null); + const rows = buildSplitRows(file, null, theme); + const plannedRows = buildReviewRenderPlan({ + fileId: file.id, + rows, + showHunkHeaders: true, + }); + const changedRow = plannedRows.find( + (row) => row.kind === "diff-row" && row.row.type === "split-line", + ); + + expect(changedRow).toBeDefined(); + if (!changedRow || changedRow.kind !== "diff-row") { + throw new Error("Expected a planned split diff row"); + } + + const [line] = renderDecoratedPlannedRowText(changedRow, { + codeHorizontalOffset: 0, + lineNumberDigits: 1, + showHunkHeaders: true, + showLineNumbers: true, + theme, + width: 80, + wrapLines: false, + }); + + expect(line).toContain("- export const answer = 41;"); + expect(line).toContain("+ export const answer = 42;"); + }); + + test("renders planned stack rows with horizontal copy offset", () => { + const file = createDiffFile(); + const theme = resolveTheme("midnight", null); + const rows = buildStackRows(file, null, theme); + const plannedRows = buildReviewRenderPlan({ + fileId: file.id, + rows, + showHunkHeaders: true, + }); + const additionRow = plannedRows.find( + (row) => + row.kind === "diff-row" && + row.row.type === "stack-line" && + row.row.cell.kind === "addition", + ); + + expect(additionRow).toBeDefined(); + if (!additionRow || additionRow.kind !== "diff-row") { + throw new Error("Expected a planned stack addition row"); + } + + const [line] = renderDecoratedPlannedRowText(additionRow, { + codeHorizontalOffset: 7, + lineNumberDigits: 1, + showHunkHeaders: true, + showLineNumbers: true, + theme, + width: 40, + wrapLines: false, + }); + + expect(line).toContain("nst answer = 42;"); + expect(line).not.toContain("export const"); + }); + + test("renders planned rows as code-only copy text when decorations are disabled", () => { + const file = createDiffFile(); + const theme = resolveTheme("midnight", null); + const rows = buildSplitRows(file, null, theme); + const plannedRows = buildReviewRenderPlan({ + fileId: file.id, + rows, + showHunkHeaders: true, + }); + const headerRow = plannedRows.find( + (row) => row.kind === "diff-row" && row.row.type === "hunk-header", + ); + const changedRow = plannedRows.find( + (row) => row.kind === "diff-row" && row.row.type === "split-line", + ); + + expect(headerRow).toBeDefined(); + expect(changedRow).toBeDefined(); + if (!headerRow || !changedRow) { + throw new Error("Expected planned header and split rows"); + } + + expect( + renderCodeOnlyPlannedRowText(headerRow, { + codeHorizontalOffset: 0, + lineNumberDigits: 1, + showHunkHeaders: true, + showLineNumbers: true, + theme, + width: 80, + wrapLines: false, + }), + ).toEqual([]); + expect( + renderCodeOnlyPlannedRowText(changedRow, { + codeHorizontalOffset: 0, + lineNumberDigits: 1, + showHunkHeaders: true, + showLineNumbers: true, + theme, + width: 80, + wrapLines: false, + }), + ).toEqual(["export const answer = 41;", "export const answer = 42;"]); + }); + test("does not produce newline characters in spans for highlighted empty lines", async () => { const file = createEmptyLineDiffFile(); const theme = resolveTheme("midnight", null); diff --git a/src/ui/diff/renderRows.tsx b/src/ui/diff/renderRows.tsx index 3576e074..21304c5a 100644 --- a/src/ui/diff/renderRows.tsx +++ b/src/ui/diff/renderRows.tsx @@ -11,13 +11,19 @@ import { diffRailMarker, dimRailColor, neutralRailColor, + selectionHighlightBg, splitCellPalette, + splitGutterText, splitLeftRailColor, splitRightRailColor, stackCellPalette, stackGutterText, stackRailColor, } from "./rowStyle"; +import { type PlannedReviewRow } from "./reviewRenderPlan"; +import { inlineNoteTitle } from "../components/panes/AgentInlineNote"; +import { wrapText } from "../lib/agentPopover"; +import type { CopySelectedRowRange } from "../components/panes/copySelection"; /** Clamp a label to one terminal row with an ellipsis. */ export function fitText(text: string, width: number) { @@ -100,45 +106,158 @@ function renderInlineSpans( fallbackBg: string, keyPrefix: string, horizontalOffset = 0, + selectionTheme?: AppTheme, + selectionColRange?: { start: number; end: number }, ) { const { spans: trimmed, usedWidth } = sliceSpansWindow(spans, horizontalOffset, width); - let padding = Math.max(0, width - usedWidth); + const needsBlending = selectionTheme && selectionColRange; - if (padding > 0) { - const lastSpan = trimmed.at(-1); + // Build the final element list by splitting spans at selection boundaries so the highlight + // applies at character-level precision rather than whole-token granularity. + const elements: ReactNode[] = []; + let colPos = 0; + let elementIndex = 0; + + for (const span of trimmed) { + const spanStart = colPos; + const spanEnd = colPos + span.text.length; + colPos = spanEnd; - // Fold trailing padding into the last span when the colors already match. - // That keeps the output identical while avoiding one extra rendered span. if ( - lastSpan && - (lastSpan.fg ?? fallbackColor) === fallbackColor && - (lastSpan.bg ?? fallbackBg) === fallbackBg + !needsBlending || + spanEnd <= selectionColRange.start || + spanStart >= selectionColRange.end ) { - lastSpan.text += " ".repeat(padding); - padding = 0; + // Span is entirely outside the selection — render with original styling. + elements.push( + + {span.text} + , + ); + continue; } - } - return ( - <> - {trimmed.map((span, index) => ( + // Compute the split offsets within this span's text. + const localSelStart = Math.max(0, selectionColRange.start - spanStart); + const localSelEnd = Math.min(span.text.length, selectionColRange.end - spanStart); + + if (localSelStart >= localSelEnd) { + // No overlap after clamping — render original. + elements.push( {span.text} - - ))} - {padding > 0 ? ( + , + ); + continue; + } + + // Split the span at selection boundaries for character-level precision. + const prefix = span.text.slice(0, localSelStart); + const selected = span.text.slice(localSelStart, localSelEnd); + const suffix = span.text.slice(localSelEnd); + + if (prefix) { + elements.push( {`${" ".repeat(padding)}`} - ) : null} - - ); + key={`${keyPrefix}:${elementIndex++}`} + fg={span.fg ?? fallbackColor} + bg={span.bg ?? fallbackBg} + > + {prefix} + , + ); + } + if (selected) { + elements.push( + + {selected} + , + ); + } + if (suffix) { + elements.push( + + {suffix} + , + ); + } + } + + // Trailing padding after all spans. + if (needsBlending) { + // Compute how much of the padding falls within the selection. + // The padding starts at colPos (which is now sum of all span text lengths = + // usedWidth after slicing) and extends to `width`. + const padStart = colPos; + const padEnd = colPos + Math.max(0, width - usedWidth); + const paddingAmount = Math.max(0, width - usedWidth); + + if (paddingAmount > 0) { + if (padStart < selectionColRange.end && padEnd > selectionColRange.start) { + // Split padding into outside/before, selected, and after. + const beforeSel = Math.max(0, selectionColRange.start - padStart); + const inSel = Math.min(paddingAmount, selectionColRange.end - padStart) - Math.max(0, beforeSel); + const afterSel = paddingAmount - beforeSel - Math.max(0, inSel); + + if (beforeSel > 0) { + elements.push( + + {" ".repeat(beforeSel)} + , + ); + } + if (inSel > 0) { + elements.push( + + {" ".repeat(inSel)} + , + ); + } + if (afterSel > 0) { + elements.push( + + {" ".repeat(afterSel)} + , + ); + } + } else { + elements.push( + + {" ".repeat(paddingAmount)} + , + ); + } + } + } else if (width - usedWidth > 0) { + // No blending — always render a separate padding span. + elements.push( + + {" ".repeat(width - usedWidth)} + , + ); + } + + return <>{elements}; } interface WrappedCellLine { @@ -148,6 +267,7 @@ interface WrappedCellLine { interface WrappedCellLayout { gutterWidth: number; + contentWidth: number; palette: ReturnType | ReturnType; lines: WrappedCellLine[]; } @@ -212,15 +332,14 @@ function buildWrappedSplitCell( showLineNumbers, prefixWidth, ); - const firstGutterText = showLineNumbers - ? `${cell.lineNumber ? String(cell.lineNumber).padStart(lineNumberDigits, " ") : " ".repeat(lineNumberDigits)} ${cell.sign}`.padEnd( - gutterWidth, - ) - : `${cell.sign} `.padEnd(gutterWidth); + const firstGutterText = splitGutterText(cell, lineNumberDigits, showLineNumbers).padEnd( + gutterWidth, + ); const wrappedSpans = wrapSpans(cell.spans, contentWidth); return { gutterWidth, + contentWidth, palette, lines: wrappedSpans.map((spans, index) => ({ gutterText: index === 0 ? firstGutterText : " ".repeat(gutterWidth), @@ -252,6 +371,7 @@ function buildWrappedStackCell( return { gutterWidth, + contentWidth, palette, lines: wrappedSpans.map((spans, index) => ({ gutterText: index === 0 ? firstGutterText : " ".repeat(gutterWidth), @@ -260,6 +380,440 @@ function buildWrappedStackCell( } satisfies WrappedCellLayout; } +/** Convert a list of spans to fixed-width plain text while preserving logical clipping. */ +function spansToPlainText(spans: RenderSpan[], width: number, horizontalOffset = 0) { + if (width <= 0) { + return ""; + } + + const visibleText = spans + .map((span) => span.text) + .join("") + .slice(Math.max(0, horizontalOffset), Math.max(0, horizontalOffset) + width); + + return visibleText.padEnd(Math.max(0, width), " "); +} + +/** Flatten styled spans to their visible text content. */ +function spansText(spans: RenderSpan[]) { + return spans.map((span) => span.text).join(""); +} + +/** Return one cell's code text without rail, gutter, sign, or line-number decorations. */ +function cellCodeText(spans: RenderSpan[], horizontalOffset = 0) { + return spansText(spans).slice(Math.max(0, horizontalOffset)); +} + +function buildPlainSplitCellLines( + cell: SplitLineCell, + width: number, + lineNumberDigits: number, + showLineNumbers: boolean, + prefixWidth: number, + theme: AppTheme, + wrapLines: boolean, + codeHorizontalOffset = 0, +) { + if (!wrapLines) { + const { gutterWidth, contentWidth } = resolveSplitCellGeometry( + width, + lineNumberDigits, + showLineNumbers, + prefixWidth, + ); + const gutterText = splitGutterText(cell, lineNumberDigits, showLineNumbers).padEnd(gutterWidth); + + return [ + { + contentWidth, + gutterWidth, + spansText: gutterText + spansToPlainText(cell.spans, contentWidth, codeHorizontalOffset), + }, + ]; + } + + const layout = buildWrappedSplitCell( + cell, + width, + lineNumberDigits, + showLineNumbers, + prefixWidth, + theme, + ); + + // Mirror the TUI renderer, which does not apply horizontal scrolling to wrapped rows. + // Keeping the plain-text path aligned avoids visual/clipboard drift. + return layout.lines.map((line) => ({ + contentWidth: layout.contentWidth, + gutterWidth: layout.gutterWidth, + spansText: line.gutterText + spansToPlainText(line.spans, layout.contentWidth), + })); +} + +function buildPlainStackCellLines( + cell: StackLineCell, + width: number, + lineNumberDigits: number, + showLineNumbers: boolean, + prefixWidth: number, + theme: AppTheme, + wrapLines: boolean, + codeHorizontalOffset = 0, +) { + if (!wrapLines) { + const { gutterWidth, contentWidth } = resolveStackCellGeometry( + width, + lineNumberDigits, + showLineNumbers, + prefixWidth, + ); + const gutterText = stackGutterText(cell, lineNumberDigits, showLineNumbers).padEnd(gutterWidth); + + return [ + { + contentWidth, + gutterWidth, + spansText: gutterText + spansToPlainText(cell.spans, contentWidth, codeHorizontalOffset), + }, + ]; + } + + const layout = buildWrappedStackCell( + cell, + width, + lineNumberDigits, + showLineNumbers, + prefixWidth, + theme, + ); + + // Mirror the TUI renderer, which does not apply horizontal scrolling to wrapped rows. + return layout.lines.map((line) => ({ + contentWidth: layout.contentWidth, + gutterWidth: layout.gutterWidth, + spansText: line.gutterText + spansToPlainText(line.spans, layout.contentWidth), + })); +} + +/** Render the marker + label that hunk-header and collapsed rows share in plain-text form. */ +function renderHeaderRowText(text: string, width: number) { + const label = fitText(text, Math.max(0, width - 1)); + return marker() + label.padEnd(Math.max(0, width - 1), " "); +} + +interface PlannedRowTextOptions { + width: number; + lineNumberDigits: number; + showLineNumbers: boolean; + showHunkHeaders: boolean; + wrapLines: boolean; + codeHorizontalOffset: number; + theme: AppTheme; + // When set, split-line rows produce text only for this side. Stack rows ignore the filter. + side?: "left" | "right"; +} + +/** Render one or more decorated plain-text lines for one planned row. */ +export function renderDecoratedPlannedRowText( + row: PlannedReviewRow, + options: PlannedRowTextOptions, +) { + const { + width, + lineNumberDigits, + showLineNumbers, + showHunkHeaders, + wrapLines, + codeHorizontalOffset, + theme, + side, + } = options; + + if (width <= 0) { + return []; + } + + if (row.kind === "inline-note") { + const title = inlineNoteTitle(row.noteIndex, row.noteCount); + const summaryLines = wrapText(row.annotation.summary ?? "", width).map((line) => + fitText(line, width), + ); + const rationaleLines = row.annotation.rationale + ? wrapText(row.annotation.rationale, width).map((line) => fitText(line, width)) + : []; + return [fitText(title, width), ...summaryLines, ...rationaleLines]; + } + + if (row.kind === "note-guide-cap") { + return [row.side === "old" ? `${"╵".padEnd(width)}` : `${" ".repeat(width - 1)}╵`]; + } + + const preparedRow = row.row; + + if (preparedRow.type === "hunk-header") { + return showHunkHeaders ? [renderHeaderRowText(preparedRow.text, width)] : []; + } + + if (preparedRow.type === "collapsed") { + return [renderHeaderRowText(`··· ${preparedRow.text} ···`, width)]; + } + + if (preparedRow.type === "split-line") { + const guideOnOldSide = row.noteGuideSide === "old"; + const guideOnNewSide = row.noteGuideSide === "new"; + const leftPrefix = guideOnOldSide ? "│" : marker(); + const rightPrefix = "▌"; + const { leftWidth, rightWidth } = resolveSplitPaneWidths(width); + const rightRenderWidth = Math.max(0, rightWidth - (guideOnNewSide ? 1 : 0)); + + const leftCell = buildPlainSplitCellLines( + preparedRow.left, + leftWidth, + lineNumberDigits, + showLineNumbers, + leftPrefix.length, + theme, + wrapLines, + codeHorizontalOffset, + ); + const rightCell = buildPlainSplitCellLines( + preparedRow.right, + rightRenderWidth, + lineNumberDigits, + showLineNumbers, + rightPrefix.length, + theme, + wrapLines, + codeHorizontalOffset, + ); + const visualLineCount = Math.max(leftCell.length, rightCell.length); + const leftGutterWidth = leftCell[0]?.gutterWidth ?? 0; + const rightGutterWidth = rightCell[0]?.gutterWidth ?? 0; + const leftPrefixPad = leftPrefix.length; + const rightPrefixPad = rightPrefix.length; + const leftContentWidth = resolvePlainContentWidth(leftWidth, leftPrefixPad, leftGutterWidth); + const rightContentWidth = resolvePlainContentWidth( + rightRenderWidth, + rightPrefixPad, + rightGutterWidth, + ); + + return Array.from({ length: visualLineCount }, (_, index) => { + const leftLine = leftCell[index] ?? { + gutterWidth: leftGutterWidth, + contentWidth: leftContentWidth, + spansText: " ".repeat(Math.max(0, leftWidth - leftPrefixPad)), + }; + const rightLine = rightCell[index] ?? { + gutterWidth: rightGutterWidth, + contentWidth: rightContentWidth, + spansText: " ".repeat(Math.max(0, rightRenderWidth - rightPrefixPad)), + }; + const normalizedLeft = ( + `${leftPrefix}${leftLine.spansText}` + + " ".repeat(Math.max(0, leftWidth - leftLine.spansText.length)) + ).slice(0, Math.max(0, leftWidth)); + const normalizedRight = ( + `${rightPrefix}${rightLine.spansText}` + + " ".repeat(Math.max(0, rightRenderWidth - rightLine.spansText.length)) + ).slice(0, Math.max(0, rightRenderWidth)); + + if (side === "left") { + return normalizedLeft; + } + if (side === "right") { + return `${normalizedRight}${guideOnNewSide ? "│" : ""}`; + } + + return `${normalizedLeft}${normalizedRight}${guideOnNewSide ? "│" : ""}`; + }); + } + + if (preparedRow.type !== "stack-line") { + return []; + } + + const guideOnOldSide = row.noteGuideSide === "old"; + const guideOnNewSide = row.noteGuideSide === "new"; + const contentWidth = Math.max(0, width - (guideOnNewSide ? 1 : 0)); + const prefix = guideOnOldSide ? "│" : marker(); + const cellLines = buildPlainStackCellLines( + preparedRow.cell, + contentWidth, + lineNumberDigits, + showLineNumbers, + prefix.length, + theme, + wrapLines, + codeHorizontalOffset, + ); + + return cellLines.map((line) => { + const visibleLine = `${prefix}${line.spansText}`; + const normalized = visibleLine.padEnd(Math.max(1, contentWidth + prefix.length), " "); + return `${normalized}${guideOnNewSide ? "│" : ""}`; + }); +} + +/** + * Render only code content for one planned row, excluding gutters, headers, notes, and filenames. + * + * Split context rows (left.text === right.text) are deduplicated to a single line because both + * sides of an unchanged context row carry identical text and shipping it twice in the clipboard + * would be noise. If the renderer ever distinguishes left/right context via styling that bleeds + * into the span text itself, this dedupe should be revisited. + */ +export function renderCodeOnlyPlannedRowText( + row: PlannedReviewRow, + options: PlannedRowTextOptions, +) { + const { + width, + lineNumberDigits, + showLineNumbers, + wrapLines, + codeHorizontalOffset, + theme, + side, + } = options; + + if (width <= 0 || row.kind !== "diff-row") { + return []; + } + + const preparedRow = row.row; + if (preparedRow.type === "hunk-header" || preparedRow.type === "collapsed") { + return []; + } + + if (preparedRow.type === "stack-line") { + if (!wrapLines) { + return [cellCodeText(preparedRow.cell.spans, codeHorizontalOffset)].filter(Boolean); + } + + return buildWrappedStackCell( + preparedRow.cell, + width, + lineNumberDigits, + showLineNumbers, + marker().length, + theme, + ) + .lines.map((line) => spansText(line.spans)) + .filter(Boolean); + } + + if (preparedRow.type !== "split-line") { + return []; + } + + if (!wrapLines) { + const leftText = + preparedRow.left.kind === "empty" + ? "" + : cellCodeText(preparedRow.left.spans, codeHorizontalOffset); + const rightText = + preparedRow.right.kind === "empty" + ? "" + : cellCodeText(preparedRow.right.spans, codeHorizontalOffset); + + if (side === "left") { + return [leftText].filter(Boolean); + } + if (side === "right") { + return [rightText].filter(Boolean); + } + + if (leftText && rightText && leftText === rightText) { + return [leftText]; + } + + return [leftText, rightText].filter(Boolean); + } + + const { leftWidth, rightWidth } = resolveSplitPaneWidths(width); + const leftLayout = buildWrappedSplitCell( + preparedRow.left, + leftWidth, + lineNumberDigits, + showLineNumbers, + marker().length, + theme, + ); + const rightLayout = buildWrappedSplitCell( + preparedRow.right, + rightWidth, + lineNumberDigits, + showLineNumbers, + 1, + theme, + ); + const visualLineCount = Math.max(leftLayout.lines.length, rightLayout.lines.length); + const lines: string[] = []; + + for (let index = 0; index < visualLineCount; index += 1) { + const leftText = + preparedRow.left.kind === "empty" ? "" : spansText(leftLayout.lines[index]?.spans ?? []); + const rightText = + preparedRow.right.kind === "empty" ? "" : spansText(rightLayout.lines[index]?.spans ?? []); + + if (side === "left") { + if (leftText) { + lines.push(leftText); + } + continue; + } + if (side === "right") { + if (rightText) { + lines.push(rightText); + } + continue; + } + + if (leftText && rightText && leftText === rightText) { + lines.push(leftText); + } else { + if (leftText) { + lines.push(leftText); + } + if (rightText) { + lines.push(rightText); + } + } + } + + return lines; +} + +/** Resolve the code content width after fixed rail and gutter columns. */ +function resolvePlainContentWidth(totalWidth: number, prefixWidth: number, gutterWidth: number) { + return Math.max(0, totalWidth - prefixWidth - gutterWidth); +} + +/** + * Apply the selection-highlight blend to a cell palette's gutter bg only. + * + * The content bg is intentionally left untouched here so renderInlineSpans can apply the same + * blend uniformly across every rendered span (including syntax-emphasis spans that supply their + * own bg). Pre-blending contentBg would cause the fallback path to double-blend. + */ +function applySelectionPalette< + P extends { gutterBg: string; contentBg: string }, +>(palette: P, theme: AppTheme): P { + return { + ...palette, + gutterBg: selectionHighlightBg(palette.gutterBg, theme), + }; +} + +/** Apply the selection-highlight blend to a prefix descriptor. */ +function applySelectionPrefix

(prefix: P, theme: AppTheme): P { + return { + ...prefix, + bg: selectionHighlightBg(prefix.bg, theme), + }; +} + /** Render one split-view cell as prefix + gutter + content spans. */ function renderSplitCell( cell: SplitLineCell, @@ -274,26 +828,37 @@ function renderSplitCell( fg: string; bg: string; }, + selected = false, + selectionColRange?: CopySelectedRowRange, + paneOffset = 0, ) { - const palette = splitCellPalette(cell.kind, theme); - const prefixWidth = prefix?.text.length ?? 0; + const basePalette = splitCellPalette(cell.kind, theme); + const palette = selected ? applySelectionPalette(basePalette, theme) : basePalette; + const resolvedPrefix = selected && prefix ? applySelectionPrefix(prefix, theme) : prefix; + const prefixWidth = resolvedPrefix?.text.length ?? 0; const { gutterWidth, contentWidth } = resolveSplitCellGeometry( width, lineNumberDigits, showLineNumbers, prefixWidth, ); - const gutterText = showLineNumbers - ? `${cell.lineNumber ? String(cell.lineNumber).padStart(lineNumberDigits, " ") : " ".repeat(lineNumberDigits)} ${cell.sign}`.padEnd( - gutterWidth, - ) - : `${cell.sign} `.padEnd(gutterWidth); + const gutterText = splitGutterText(cell, lineNumberDigits, showLineNumbers).padEnd(gutterWidth); + + // Convert global selection column range to content-local range. + const globalContentStart = paneOffset + prefixWidth + gutterWidth; + const localColRange = + selectionColRange && globalContentStart < selectionColRange.endCol + ? { + start: Math.max(0, selectionColRange.startCol - globalContentStart), + end: Math.min(contentWidth, Math.max(0, selectionColRange.endCol - globalContentStart + 1)), + } + : undefined; return ( <> - {prefix ? ( - - {prefix.text} + {resolvedPrefix ? ( + + {resolvedPrefix.text} ) : null} @@ -306,6 +871,8 @@ function renderSplitCell( palette.contentBg, `${keyPrefix}:content`, contentOffset, + selected ? theme : undefined, + localColRange, )} ); @@ -325,9 +892,13 @@ function renderStackCell( fg: string; bg: string; }, + selected = false, + selectionColRange?: CopySelectedRowRange, ) { - const palette = stackCellPalette(cell.kind, theme); - const prefixWidth = prefix?.text.length ?? 0; + const basePalette = stackCellPalette(cell.kind, theme); + const palette = selected ? applySelectionPalette(basePalette, theme) : basePalette; + const resolvedPrefix = selected && prefix ? applySelectionPrefix(prefix, theme) : prefix; + const prefixWidth = resolvedPrefix?.text.length ?? 0; const { gutterWidth, contentWidth } = resolveStackCellGeometry( width, lineNumberDigits, @@ -335,11 +906,21 @@ function renderStackCell( prefixWidth, ); + // Convert global selection column range to content-local range. + const globalContentStart = prefixWidth + gutterWidth; + const localColRange = + selectionColRange && globalContentStart < selectionColRange.endCol + ? { + start: Math.max(0, selectionColRange.startCol - globalContentStart), + end: Math.min(contentWidth, Math.max(0, selectionColRange.endCol - globalContentStart + 1)), + } + : undefined; + return ( <> - {prefix ? ( - - {prefix.text} + {resolvedPrefix ? ( + + {resolvedPrefix.text} ) : null} @@ -352,6 +933,8 @@ function renderStackCell( palette.contentBg, `${keyPrefix}:content`, contentOffset, + selected ? theme : undefined, + localColRange, )} ); @@ -369,21 +952,45 @@ function renderWrappedSplitCellLine( fg: string; bg: string; }, + selected = false, + selectionColRange?: CopySelectedRowRange, + paneOffset = 0, ) { + const resolvedPalette = selected ? applySelectionPalette(palette, theme) : palette; + const resolvedPrefix = selected ? applySelectionPrefix(prefix, theme) : prefix; + + const prefixWidth = prefix.text.length; + const gutterWidth = line.gutterText.length; + const globalContentStart = paneOffset + prefixWidth + gutterWidth; + const localColRange = + selectionColRange && globalContentStart < selectionColRange.endCol + ? { + start: Math.max(0, selectionColRange.startCol - globalContentStart), + end: Math.min(contentWidth, Math.max(0, selectionColRange.endCol - globalContentStart + 1)), + } + : undefined; + return ( <> - - {prefix.text} + + {resolvedPrefix.text} - + {line.gutterText} {renderInlineSpans( line.spans, contentWidth, theme.text, - palette.contentBg, + resolvedPalette.contentBg, `${keyPrefix}:content`, + 0, + selected ? theme : undefined, + localColRange, )} ); @@ -401,21 +1008,44 @@ function renderWrappedStackCellLine( fg: string; bg: string; }, + selected = false, + selectionColRange?: CopySelectedRowRange, ) { + const resolvedPalette = selected ? applySelectionPalette(palette, theme) : palette; + const resolvedPrefix = selected ? applySelectionPrefix(prefix, theme) : prefix; + + const prefixWidth = prefix.text.length; + const gutterWidth = line.gutterText.length; + const globalContentStart = prefixWidth + gutterWidth; + const localColRange = + selectionColRange && globalContentStart < selectionColRange.endCol + ? { + start: Math.max(0, selectionColRange.startCol - globalContentStart), + end: Math.min(contentWidth, Math.max(0, selectionColRange.endCol - globalContentStart + 1)), + } + : undefined; + return ( <> - - {prefix.text} + + {resolvedPrefix.text} - + {line.gutterText} {renderInlineSpans( line.spans, contentWidth, theme.text, - palette.contentBg, + resolvedPalette.contentBg, `${keyPrefix}:content`, + 0, + selected ? theme : undefined, + localColRange, )} ); @@ -604,11 +1234,20 @@ function renderRow( codeHorizontalOffset: number, theme: AppTheme, selected: boolean, + copySelectedRowRange: CopySelectedRowRange | undefined, + copySelectedSide: "left" | "right" | undefined, annotated: boolean, anchorId?: string, noteGuideSide?: "old" | "new", onOpenAgentNotesAtHunk?: (hunkIndex: number) => void, ) { + const hasCopySelection = !!copySelectedRowRange; + + // For split rows, the user's drag is anchored to one column-half of the diff. Apply the + // selection-highlight blend only to that side so it is clear which file (A or B) the + // selection represents. + const hasLeftSelection = hasCopySelection && copySelectedSide !== "right"; + const hasRightSelection = hasCopySelection && copySelectedSide !== "left"; let baseRow: ReactNode; if (row.type === "collapsed") { @@ -616,14 +1255,22 @@ function renderRow( row, width, theme, - selected, + selected || hasCopySelection, annotated, anchorId, onOpenAgentNotesAtHunk, ); } else if (row.type === "hunk-header") { baseRow = showHunkHeaders - ? renderHeaderRow(row, width, theme, selected, annotated, anchorId, onOpenAgentNotesAtHunk) + ? renderHeaderRow( + row, + width, + theme, + selected || hasCopySelection, + annotated, + anchorId, + onOpenAgentNotesAtHunk, + ) : null; } else if (row.type === "split-line") { const guideOnOldSide = noteGuideSide === "old"; @@ -634,12 +1281,14 @@ function renderRow( const rightRenderWidth = Math.max(0, rightWidth - (guideOnNewSide ? 1 : 0)); const leftPrefix = { text: guideOnOldSide ? "│" : marker(), - fg: guideOnOldSide ? theme.noteBorder : splitLeftRailColor(row.left.kind, theme, selected), + fg: guideOnOldSide + ? theme.noteBorder + : splitLeftRailColor(row.left.kind, theme, selected || hasCopySelection), bg: theme.panel, }; const rightPrefix = { text: "▌", - fg: splitRightRailColor(row.right.kind, theme, selected), + fg: splitRightRailColor(row.right.kind, theme, selected || hasCopySelection), bg: theme.panel, }; @@ -656,6 +1305,9 @@ function renderRow( `${row.key}:left`, codeHorizontalOffset, leftPrefix, + hasLeftSelection, + hasLeftSelection ? copySelectedRowRange : undefined, + 0, )} {renderSplitCell( row.right, @@ -666,6 +1318,9 @@ function renderRow( `${row.key}:right`, codeHorizontalOffset, rightPrefix, + hasRightSelection, + hasRightSelection ? copySelectedRowRange : undefined, + leftWidth, )} {guideOnNewSide ? ( @@ -724,6 +1379,9 @@ function renderRow( theme, `${row.key}:left:${index}`, leftPrefix, + hasLeftSelection, + hasLeftSelection ? copySelectedRowRange : undefined, + 0, )} {renderWrappedSplitCellLine( rightLine, @@ -732,6 +1390,9 @@ function renderRow( theme, `${row.key}:right:${index}`, rightPrefix, + hasRightSelection, + hasRightSelection ? copySelectedRowRange : undefined, + leftWidth, )} {guideOnNewSide ? ( @@ -751,7 +1412,9 @@ function renderRow( const contentWidth = Math.max(0, width - (guideOnNewSide ? 1 : 0)); const prefix = { text: guideOnOldSide ? "│" : marker(), - fg: guideOnOldSide ? theme.noteBorder : stackRailColor(row.cell.kind, theme, selected), + fg: guideOnOldSide + ? theme.noteBorder + : stackRailColor(row.cell.kind, theme, selected || hasCopySelection), bg: theme.panel, }; @@ -768,6 +1431,8 @@ function renderRow( `${row.key}:stack`, codeHorizontalOffset, prefix, + hasCopySelection, + hasCopySelection ? copySelectedRowRange : undefined, )} {guideOnNewSide ? ( @@ -803,6 +1468,8 @@ function renderRow( theme, `${row.key}:stack:${index}`, prefix, + hasCopySelection, + hasCopySelection ? copySelectedRowRange : undefined, )} {guideOnNewSide ? ( @@ -836,6 +1503,8 @@ interface DiffRowViewProps { codeHorizontalOffset: number; theme: AppTheme; selected: boolean; + copySelectedRowRange?: CopySelectedRowRange; + copySelectedSide?: "left" | "right"; annotated: boolean; anchorId?: string; noteGuideSide?: "old" | "new"; @@ -854,6 +1523,8 @@ export const DiffRowView = memo( codeHorizontalOffset, theme, selected, + copySelectedRowRange, + copySelectedSide, annotated, anchorId, noteGuideSide, @@ -869,6 +1540,8 @@ export const DiffRowView = memo( codeHorizontalOffset, theme, selected, + copySelectedRowRange, + copySelectedSide, annotated, anchorId, noteGuideSide, @@ -886,6 +1559,8 @@ export const DiffRowView = memo( previous.codeHorizontalOffset === next.codeHorizontalOffset && previous.theme === next.theme && previous.selected === next.selected && + previous.copySelectedRowRange === next.copySelectedRowRange && + previous.copySelectedSide === next.copySelectedSide && previous.annotated === next.annotated && previous.anchorId === next.anchorId && previous.noteGuideSide === next.noteGuideSide diff --git a/src/ui/diff/rowStyle.ts b/src/ui/diff/rowStyle.ts index 74f81bad..7170295b 100644 --- a/src/ui/diff/rowStyle.ts +++ b/src/ui/diff/rowStyle.ts @@ -3,12 +3,24 @@ import { blendHex } from "../lib/color"; import type { SplitLineCell, StackLineCell } from "./pierre"; const INACTIVE_RAIL_BLEND = 0.35; +const SELECTION_BG_BLEND = 0.75; /** The diff rail marker is always visible in Hunk stack and split rows. */ export function diffRailMarker() { return "▌"; } +/** + * Blend a base cell background toward the selection highlight color. + * + * blendHex(fg, bg, ratio) returns `bg + (fg - bg) * ratio`. We pass the highlight color as the + * "front" and the cell's base bg as the "back", so a higher SELECTION_BG_BLEND pulls the result + * harder toward the visible highlight color. + */ +export function selectionHighlightBg(baseBg: string, theme: AppTheme) { + return blendHex(theme.selectedHunk, baseBg, SELECTION_BG_BLEND); +} + /** Return the neutral active-hunk rail color for the current theme. */ export function neutralRailColor(theme: AppTheme) { return theme.lineNumberFg; @@ -138,3 +150,19 @@ export function stackGutterText( const newNumber = diffLineNumberText(cell.newLineNumber, lineNumberDigits); return `${oldNumber} ${newNumber} ${cell.sign}`; } + +/** Build the split-view gutter text shared by the TUI and clipboard renderers. */ +export function splitGutterText( + cell: SplitLineCell, + lineNumberDigits: number, + showLineNumbers: boolean, +) { + if (!showLineNumbers) { + return `${cell.sign} `; + } + + const number = cell.lineNumber + ? String(cell.lineNumber).padStart(lineNumberDigits, " ") + : " ".repeat(lineNumberDigits); + return `${number} ${cell.sign}`; +} diff --git a/src/ui/diff/rowWindowing.test.ts b/src/ui/diff/rowWindowing.test.ts index 3dd47c38..47b23679 100644 --- a/src/ui/diff/rowWindowing.test.ts +++ b/src/ui/diff/rowWindowing.test.ts @@ -36,6 +36,8 @@ function createTestSectionGeometry( bodyHeight, hunkAnchorRows: new Map(), hunkBounds: new Map(), + lineNumberDigits: 1, + plannedRows: rowBounds.map((row) => createTestPlannedRow(row.key)), rowBounds: normalizedRowBounds, rowBoundsByKey: new Map(normalizedRowBounds.map((row) => [row.key, row])), rowBoundsByStableKey: new Map(normalizedRowBounds.map((row) => [row.stableKey, row])), diff --git a/src/ui/lib/appMenus.ts b/src/ui/lib/appMenus.ts index 058bb33c..d07429d8 100644 --- a/src/ui/lib/appMenus.ts +++ b/src/ui/lib/appMenus.ts @@ -14,11 +14,13 @@ export interface BuildAppMenusOptions { requestQuit: () => void; selectLayoutMode: (mode: LayoutMode) => void; selectThemeId: (themeId: string) => void; + copyDecorations: boolean; showAgentNotes: boolean; showHelp: boolean; showHunkHeaders: boolean; showLineNumbers: boolean; renderSidebar: boolean; + toggleCopyDecorations: () => void; toggleAgentNotes: () => void; toggleFocusArea: () => void; toggleHelp: () => void; @@ -42,11 +44,13 @@ export function buildAppMenus({ requestQuit, selectLayoutMode, selectThemeId, + copyDecorations, showAgentNotes, showHelp, showHunkHeaders, showLineNumbers, renderSidebar, + toggleCopyDecorations, toggleAgentNotes, toggleFocusArea, toggleHelp, @@ -158,6 +162,12 @@ export function buildAppMenus({ checked: showHunkHeaders, action: toggleHunkHeaders, }, + { + kind: "item", + label: "Copy decorations", + checked: copyDecorations, + action: toggleCopyDecorations, + }, ], navigate: [ { diff --git a/src/ui/lib/diffSectionGeometry.ts b/src/ui/lib/diffSectionGeometry.ts index f97ad63e..2dfd2f62 100644 --- a/src/ui/lib/diffSectionGeometry.ts +++ b/src/ui/lib/diffSectionGeometry.ts @@ -16,25 +16,29 @@ export interface DiffSectionRowBounds extends VerticalBounds { stableKeys: string[]; } -/** Cached placeholder sizing and hunk navigation geometry for one file section. */ +/** + * Cached placeholder sizing and hunk navigation geometry for one file section. + * + * `plannedRows` is retained alongside the row-bounds map so downstream features (notably + * clipboard rendering of mouse selections) can re-render the exact same rows the layout was + * measured against, without rebuilding the plan. The cache is keyed off the agent-notes input + * via a WeakMap so memory grows in step with the visible diff, not per-render. + */ export interface DiffSectionGeometry extends SectionGeometry { + lineNumberDigits: number; + plannedRows: PlannedReviewRow[]; rowBounds: DiffSectionRowBounds[]; rowBoundsByKey: Map; rowBoundsByStableKey: Map; } -const NOTE_AWARE_SECTION_GEOMETRY_CACHE = new WeakMap< - VisibleAgentNote[], - Map ->(); - -/** Build the exact review rows for one file before converting them into section geometry. */ -function buildBasePlannedRows( +/** Build the planned row stream for one file section using the same shape as geometry measurement. */ +function buildPlannedSectionRows( file: DiffFile, layout: Exclude, showHunkHeaders: boolean, theme: AppTheme, - visibleAgentNotes: VisibleAgentNote[], + visibleAgentNotes: VisibleAgentNote[] = [], ) { const rows = layout === "split" ? buildSplitRows(file, null, theme) : buildStackRows(file, null, theme); @@ -42,12 +46,16 @@ function buildBasePlannedRows( return buildReviewRenderPlan({ fileId: file.id, rows, - selectedHunkIndex: -1, showHunkHeaders, visibleAgentNotes, }); } +const NOTE_AWARE_SECTION_GEOMETRY_CACHE = new WeakMap< + VisibleAgentNote[], + Map +>(); + /** Measure how many terminal rows one planned review row occupies for the current view settings. */ function plannedRowHeight( row: PlannedReviewRow, @@ -106,6 +114,8 @@ export function measureDiffSectionGeometry( bodyHeight: 1, hunkAnchorRows: new Map(), hunkBounds: new Map(), + lineNumberDigits: String(findMaxLineNumber(file)).length, + plannedRows: [], rowBounds: [], rowBoundsByKey: new Map(), rowBoundsByStableKey: new Map(), @@ -123,7 +133,13 @@ export function measureDiffSectionGeometry( } } - const plannedRows = buildBasePlannedRows(file, layout, showHunkHeaders, theme, visibleAgentNotes); + const plannedRows = buildPlannedSectionRows( + file, + layout, + showHunkHeaders, + theme, + visibleAgentNotes, + ); const hunkAnchorRows = new Map(); const hunkBounds = new Map(); const rowBounds: DiffSectionRowBounds[] = []; @@ -192,6 +208,8 @@ export function measureDiffSectionGeometry( bodyHeight, hunkAnchorRows, hunkBounds, + lineNumberDigits, + plannedRows, rowBounds, rowBoundsByKey, rowBoundsByStableKey, @@ -205,7 +223,6 @@ export function measureDiffSectionGeometry( return geometry; } - /** Estimate the number of diff-body rows for one file section in the windowed path. */ export function estimateDiffSectionBodyRows( file: DiffFile, diff --git a/src/ui/lib/ui-lib.test.ts b/src/ui/lib/ui-lib.test.ts index 14136c59..ac094afd 100644 --- a/src/ui/lib/ui-lib.test.ts +++ b/src/ui/lib/ui-lib.test.ts @@ -132,11 +132,13 @@ describe("ui helpers", () => { requestQuit: () => {}, selectLayoutMode: () => {}, selectThemeId: () => {}, + copyDecorations: true, showAgentNotes: true, showHelp: false, showHunkHeaders: false, showLineNumbers: true, renderSidebar: false, + toggleCopyDecorations: () => {}, toggleAgentNotes: () => {}, toggleFocusArea: () => {}, toggleHelp: () => {}, @@ -164,7 +166,7 @@ describe("ui helpers", () => { entry.kind === "item" && Boolean(entry.checked), ) .map((entry) => entry.label), - ).toEqual(["Stacked view", "Agent notes", "Line numbers", "Line wrapping"]); + ).toEqual(["Stacked view", "Agent notes", "Line numbers", "Line wrapping", "Copy decorations"]); expect( menus.theme .filter((entry): entry is Extract => entry.kind === "item") diff --git a/src/ui/themes.ts b/src/ui/themes.ts index 38ad3883..394f77fc 100644 --- a/src/ui/themes.ts +++ b/src/ui/themes.ts @@ -111,7 +111,7 @@ export const THEMES: AppTheme[] = [ removedSignColor: "#f0a0a0", lineNumberBg: "#14181b", lineNumberFg: "#798592", - selectedHunk: "#3b434b", + selectedHunk: "#4f5d6b", badgeAdded: "#88d39b", badgeRemoved: "#f0a0a0", badgeNeutral: "#a9b4bf", @@ -160,7 +160,7 @@ export const THEMES: AppTheme[] = [ removedSignColor: "#ff8e8e", lineNumberBg: "#0b1627", lineNumberFg: "#56739a", - selectedHunk: "#20466a", + selectedHunk: "#2a6a8a", badgeAdded: "#5ad188", badgeRemoved: "#ff8b8b", badgeNeutral: "#89a5d3", @@ -209,7 +209,7 @@ export const THEMES: AppTheme[] = [ removedSignColor: "#b4545b", lineNumberBg: "#f2e9dc", lineNumberFg: "#9b8367", - selectedHunk: "#eadcc5", + selectedHunk: "#d2c0a5", badgeAdded: "#3f8d58", badgeRemoved: "#b4545b", badgeNeutral: "#8e7355", @@ -258,7 +258,7 @@ export const THEMES: AppTheme[] = [ removedSignColor: "#ff9d8f", lineNumberBg: "#1c100c", lineNumberFg: "#9a735f", - selectedHunk: "#6a3829", + selectedHunk: "#8a4d3a", badgeAdded: "#83d99d", badgeRemoved: "#ff9d8f", badgeNeutral: "#f1be9d", diff --git a/test/helpers/app-bootstrap.ts b/test/helpers/app-bootstrap.ts index 29b81f33..d49cb88f 100644 --- a/test/helpers/app-bootstrap.ts +++ b/test/helpers/app-bootstrap.ts @@ -6,6 +6,7 @@ export function createTestVcsAppBootstrap({ files, vcsOptions = {}, initialMode = "split", + initialCopyDecorations, initialShowAgentNotes, initialShowHunkHeaders, initialShowLineNumbers, @@ -22,6 +23,7 @@ export function createTestVcsAppBootstrap({ files: DiffFile[]; vcsOptions?: Partial; initialMode?: LayoutMode; + initialCopyDecorations?: boolean; initialShowAgentNotes?: boolean; initialShowHunkHeaders?: boolean; initialShowLineNumbers?: boolean; @@ -52,6 +54,7 @@ export function createTestVcsAppBootstrap({ title, }, initialMode, + initialCopyDecorations, initialShowAgentNotes, initialShowHunkHeaders, initialShowLineNumbers, From eb8441c329a4ba19b49c84631c478c905e6e7ef8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Thu, 14 May 2026 19:19:38 +0100 Subject: [PATCH 2/2] fix(ui): address PR review feedback on copy selection - Fix copyDecorations defaulting to true instead of false so new users get code-only clipboard output by default (P1) - Fix double-click on whitespace producing inverted column range by selecting the whitespace character itself (P2) - Fix stale copySelectionDrag closure on first drag event causing brief native-selection flash by mirroring drag state in a ref (P2) - Fix bare setTimeout in showTransientNotice that could fire after unmount by tracking the timer in a ref and clearing on unmount (P2) - Add test coverage for whitespace double-click edge case --- src/core/config.ts | 2 +- src/ui/App.tsx | 17 +++++++- src/ui/components/panes/DiffPane.tsx | 40 +++++++++++++++---- src/ui/components/panes/copySelection.test.ts | 36 +++++++++++++++++ src/ui/components/panes/copySelection.ts | 11 +++++ 5 files changed, 97 insertions(+), 9 deletions(-) diff --git a/src/core/config.ts b/src/core/config.ts index 24438c93..31179c27 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -15,7 +15,7 @@ const DEFAULT_VIEW_PREFERENCES: PersistedViewPreferences = { wrapLines: false, showHunkHeaders: true, showAgentNotes: false, - copyDecorations: true, + copyDecorations: false, }; interface ConfigResolutionOptions { diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 7bc3e945..d8965955 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -304,13 +304,28 @@ export function App({ // Show a short-lived status-bar message. Used to surface clipboard-copy outcomes that would // otherwise be invisible to the user (OSC52 unsupported, etc.). + // Track the timer so we can clear it on unmount and avoid React state updates after unmount. + const transientTimerRef = useRef | null>(null); const showTransientNotice = useCallback((text: string, durationMs = 3000) => { + if (transientTimerRef.current !== null) { + clearTimeout(transientTimerRef.current); + } setTransientNoticeText(text); - setTimeout(() => { + transientTimerRef.current = setTimeout(() => { + transientTimerRef.current = null; setTransientNoticeText((current) => (current === text ? null : current)); }, durationMs); }, []); + // Clear any pending transient-notice timer on unmount to avoid state updates after unmount. + useEffect(() => { + return () => { + if (transientTimerRef.current !== null) { + clearTimeout(transientTimerRef.current); + } + }; + }, []); + /** Toggle whether diff code rows wrap instead of truncating to one terminal row. */ const toggleLineWrap = () => { // Capture the pre-toggle viewport position synchronously so DiffPane can restore the same diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 9b0253f6..d9ee2777 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -306,6 +306,9 @@ export function DiffPane({ const windowingEnabled = !wrapLines; const [scrollViewport, setScrollViewport] = useState({ top: 0, height: 0 }); const [copySelectionDrag, setCopySelectionDrag] = useState(null); + // Mirror the drag state in a ref so updateCopySelection can suppress native selection + // on the very first drag event, before React has re-rendered with the new state. + const copySelectionDragRef = useRef(null); const lastClickTimeRef = useRef(0); const clickCountRef = useRef(0); const scrollbarRef = useRef(null); @@ -695,6 +698,7 @@ export function DiffPane({ const point = resolveCopySelectionPoint(event); if (!point) { + copySelectionDragRef.current = null; setCopySelectionDrag(null); return; } @@ -719,11 +723,13 @@ export function DiffPane({ copySelectionContext, ); if (expanded) { - setCopySelectionDrag({ + const drag: CopySelectionDrag = { anchor: { ...point, column: expanded.startCol }, focus: { ...point, column: expanded.endCol }, moved: true, - }); + }; + copySelectionDragRef.current = drag; + setCopySelectionDrag(drag); suppressNativeSelection(); event.preventDefault(); event.stopPropagation(); @@ -731,7 +737,9 @@ export function DiffPane({ } } - setCopySelectionDrag({ anchor: point, focus: point, moved: false }); + const initial: CopySelectionDrag = { anchor: point, focus: point, moved: false }; + copySelectionDragRef.current = initial; + setCopySelectionDrag(initial); suppressNativeSelection(); event.preventDefault(); event.stopPropagation(); @@ -742,6 +750,8 @@ export function DiffPane({ /** Extend the active diff text selection while the pointer moves. */ const updateCopySelection = useCallback( (event: TuiMouseEvent) => { + // Use the ref (not state) so that native-selection suppression fires on the very + // first drag event, before React has re-rendered with the new copySelectionDrag. setCopySelectionDrag((current) => { if (!current) { return current; @@ -759,23 +769,39 @@ export function DiffPane({ }; }); - if (copySelectionDrag) { + // The state updater above sets the ref during the render phase. Update the ref + // synchronously as well so that endCopySelection can read the correct moved flag + // even if the mouse-up event fires before React processes the pending state update. + const refDrag = copySelectionDragRef.current; + if (refDrag) { + const point = resolveCopySelectionPoint(event); + if (point) { + copySelectionDragRef.current = { + anchor: refDrag.anchor, + focus: point, + moved: refDrag.moved || !copySelectionPointsEqual(point, refDrag.anchor), + }; + } + } + + if (copySelectionDragRef.current) { suppressNativeSelection(); event.preventDefault(); event.stopPropagation(); } }, - [copySelectionDrag, resolveCopySelectionPoint, suppressNativeSelection], + [resolveCopySelectionPoint, suppressNativeSelection], ); /** Finish a drag selection and copy its rendered text. */ const endCopySelection = useCallback( (event?: TuiMouseEvent) => { - const current = copySelectionDrag; + const current = copySelectionDragRef.current; if (!current) { return; } + copySelectionDragRef.current = null; setCopySelectionDrag(null); event?.preventDefault(); event?.stopPropagation(); @@ -793,7 +819,7 @@ export function DiffPane({ }); copySelectionText(text); }, - [copySelectionDrag, copySelectionContext, copySelectionSide, copySelectionText], + [copySelectionContext, copySelectionSide, copySelectionText], ); // Expose the cancel hook so an ancestor (App's outer container) can release a stuck drag when diff --git a/src/ui/components/panes/copySelection.test.ts b/src/ui/components/panes/copySelection.test.ts index 5d39972a..15825c24 100644 --- a/src/ui/components/panes/copySelection.test.ts +++ b/src/ui/components/panes/copySelection.test.ts @@ -20,6 +20,7 @@ import { import { DIFF_RAIL_PREFIX_WIDTH, resolveSplitCellGeometry, + resolveStackCellGeometry, resolveSplitPaneWidths, } from "../../diff/codeColumns"; @@ -430,6 +431,41 @@ describe("expandSelectionPoint", () => { expect(side).toBe("right"); } }); + + test("double-click on whitespace selects the whitespace character itself", () => { + const { context, fileSectionLayouts, sectionGeometry } = buildContext("stack"); + const section = fileSectionLayouts[0]!; + const geometry = sectionGeometry[0]!; + + // Compute the global column for the space character between "export" and "const". + // The addition row "export const answer = 42;" starts at bodyTop + 2 + // (after a hunk header row and a deletion row). + const { gutterWidth } = resolveStackCellGeometry( + context.width, + geometry.lineNumberDigits, + context.showLineNumbers, + DIFF_RAIL_PREFIX_WIDTH, + ); + const globalContentStart = DIFF_RAIL_PREFIX_WIDTH + gutterWidth; + // "export" is 6 chars, so the space after it is at code-local column 6. + const spaceCol = globalContentStart + 6; + + const point: CopySelectionPoint = { + kind: "review-row", + column: spaceCol, + visualRow: section.bodyTop + 2, // addition row: "export const answer = 42;" + }; + + const result = expandSelectionPoint(point, 2, context); + expect(result).not.toBeNull(); + if (result) { + // startCol and endCol should be equal (single whitespace character), + // never inverted (endCol < startCol). + expect(result.startCol).toBeLessThanOrEqual(result.endCol); + expect(result.startCol).toBe(spaceCol); + expect(result.endCol).toBe(spaceCol); + } + }); }); describe("renderCopySelectionText in split with side", () => { diff --git a/src/ui/components/panes/copySelection.ts b/src/ui/components/panes/copySelection.ts index a0c51f13..325f489c 100644 --- a/src/ui/components/panes/copySelection.ts +++ b/src/ui/components/panes/copySelection.ts @@ -467,6 +467,17 @@ export function expandSelectionPoint( // Convert the global click column to a code-local column. const localCol = Math.max(0, Math.min(lineText.length - 1, point.column - globalContentStart)); + + // When double-clicking on whitespace, the word loops below don't advance, + // producing an inverted range (endCol < startCol). Select just the + // whitespace character itself instead. + if (lineText[localCol] === " " || lineText[localCol] === "\t") { + return { + startCol: localCol + globalContentStart, + endCol: localCol + globalContentStart, + }; + } + let wordStart = localCol; let wordEnd = localCol;