diff --git a/src/renderer/src/components/editor/DiffSectionItem.tsx b/src/renderer/src/components/editor/DiffSectionItem.tsx index 2fc40c16cba..8847bad5fdc 100644 --- a/src/renderer/src/components/editor/DiffSectionItem.tsx +++ b/src/renderer/src/components/editor/DiffSectionItem.tsx @@ -87,16 +87,23 @@ export function DiffSectionItem({ const fileDiff = useMemo( () => - buildPierreFileDiff({ - path: section.path, - oldPath: section.oldPath, - status: section.status, - originalContent: section.originalContent, - modifiedContent: section.modifiedContent, - // Why: keyed by content generation so the worker AST cache survives virtualization remounts. - cacheKey: `${section.key}:${section.contentGeneration ?? 0}`, - parseDiffOptions: buildPierreParseDiffOptions(settings?.diffShowWhitespace) - }), + section.collapsed || + section.loading || + section.loadOnDemand || + section.error || + section.diffResult?.kind === 'binary' || + section.largeDiffRenderLimit?.limited + ? null + : buildPierreFileDiff({ + path: section.path, + oldPath: section.oldPath, + status: section.status, + originalContent: section.originalContent, + modifiedContent: section.modifiedContent, + // Why: keyed by content generation so the worker AST cache survives virtualization remounts. + cacheKey: `${worktreeId}:${section.key}:${section.contentGeneration ?? 0}`, + parseDiffOptions: buildPierreParseDiffOptions(settings?.diffShowWhitespace) + }), [ section.path, section.oldPath, @@ -105,6 +112,13 @@ export function DiffSectionItem({ section.modifiedContent, section.key, section.contentGeneration, + section.collapsed, + section.loading, + section.loadOnDemand, + section.error, + section.diffResult?.kind, + section.largeDiffRenderLimit?.limited, + worktreeId, settings?.diffShowWhitespace ] ) @@ -209,32 +223,33 @@ export function DiffSectionItem({ return } return installEditorSaveShortcut(node, () => void handleSectionSaveRef.current(index)) - }, [handleSectionSaveRef, index, isEditable]) + }, [handleSectionSaveRef, index, isEditable, section.collapsed]) const renderDiff = useCallback( - () => ( - setPendingComment(null)} - onSubmitComment={handleSubmitComment} - /> - ), + () => + fileDiff ? ( + setPendingComment(null)} + onSubmitComment={handleSubmitComment} + /> + ) : null, [ addLineCommentLabel, addLineCommentPlaceholder, diff --git a/src/renderer/src/components/editor/DiffViewer.tsx b/src/renderer/src/components/editor/DiffViewer.tsx index 0f062645bd9..fd46446764a 100644 --- a/src/renderer/src/components/editor/DiffViewer.tsx +++ b/src/renderer/src/components/editor/DiffViewer.tsx @@ -17,6 +17,7 @@ import { PierreDiffSurface } from './pierre-diff/PierreDiffSurface' import { buildPierreFileDiff } from './pierre-diff/pierre-diff-metadata' import { buildPierreParseDiffOptions } from './pierre-diff/pierre-diff-options' import { scrollPierreDiffToLine } from './pierre-diff/pierre-diff-scroll' +import { getPierreDiffChangeTargets } from './pierre-diff/pierre-diff-change-targets' const EMPTY_DIFF_COMMENTS: readonly DecoratedDiffComment[] = [] @@ -70,19 +71,32 @@ export default function DiffViewer({ const fileDiff = useMemo( () => - buildPierreFileDiff({ - path: relativePath, - status: 'modified', - originalContent, - modifiedContent, - cacheKey: modelKey, - parseDiffOptions: buildPierreParseDiffOptions(settings?.diffShowWhitespace) - }), - [relativePath, originalContent, modifiedContent, modelKey, settings?.diffShowWhitespace] + renderLimit.limited + ? null + : buildPierreFileDiff({ + path: relativePath, + status: 'modified', + originalContent, + modifiedContent, + cacheKey: modelKey, + parseDiffOptions: buildPierreParseDiffOptions(settings?.diffShowWhitespace) + }), + [ + relativePath, + originalContent, + modifiedContent, + modelKey, + settings?.diffShowWhitespace, + renderLimit.limited + ] ) const { registerDiffNavigator, unregisterDiffNavigator } = useDiffNavigatorRegistration() - const changeLines = useMemo(() => fileDiff.hunks.map((hunk) => hunk.additionStart), [fileDiff]) + const changeTargets = useMemo(() => getPierreDiffChangeTargets(fileDiff), [fileDiff]) + const changeLines = useMemo( + () => changeTargets.map((target) => target.lineNumber), + [changeTargets] + ) useEffect(() => { const container = scrollContainerRef.current @@ -97,6 +111,7 @@ export default function DiffViewer({ host: pierreHostRef.current, container, lineNumber, + side: changeTargets[hunkIndex]?.side, hunkIndex, hunkCount }) @@ -104,7 +119,13 @@ export default function DiffViewer({ } registerDiffNavigator(navigator) return () => unregisterDiffNavigator(navigator) - }, [changeLines, registerDiffNavigator, renderLimit.limited, unregisterDiffNavigator]) + }, [ + changeLines, + changeTargets, + registerDiffNavigator, + renderLimit.limited, + unregisterDiffNavigator + ]) const handlePostRender = useCallback((node: HTMLElement, phase: PostRenderPhase) => { pierreHostRef.current = phase === 'unmount' ? null : node @@ -243,7 +264,7 @@ export default function DiffViewer({ saveContentAvailable: largeDiffSaveContentAvailable })} /> - ) : ( + ) : fileDiff ? ( - )} + ) : null} ) diff --git a/src/renderer/src/components/editor/diff-section-live-render-limit.ts b/src/renderer/src/components/editor/diff-section-live-render-limit.ts index 2a4ad6ba1e4..50ad082c731 100644 --- a/src/renderer/src/components/editor/diff-section-live-render-limit.ts +++ b/src/renderer/src/components/editor/diff-section-live-render-limit.ts @@ -1,6 +1,7 @@ import type { DiffSection } from './diff-section-types' import { getLargeDiffRenderLimitFromCounts, + countLinesEmptyAsZero, type LargeDiffRenderLimit } from './large-diff-render-limit' @@ -12,10 +13,12 @@ export function getLiveDiffSectionRenderLimit({ modifiedContent: string }): LargeDiffRenderLimit { // Why: the renderer no longer owns a text model, so count lines from the draft itself. - const modifiedLineCount = modifiedContent.length === 0 ? 0 : modifiedContent.split('\n').length + const modifiedLineCount = countLinesEmptyAsZero(modifiedContent) return getLargeDiffRenderLimitFromCounts({ - originalLineCount: section.largeDiffRenderLimit?.lineCounts?.original ?? 0, + originalLineCount: + section.largeDiffRenderLimit?.lineCounts?.original ?? + countLinesEmptyAsZero(section.originalContent), modifiedLineCount, originalCharacterCount: section.originalContent.length, modifiedCharacterCount: modifiedContent.length diff --git a/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx index 1f3167452a9..303bc6ff584 100644 --- a/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx +++ b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx @@ -1,7 +1,11 @@ import { useCallback, useEffect, useMemo, useRef } from 'react' import { FileDiff } from '@pierre/diffs/react' -import type { FileDiffMetadata, PostRenderPhase, SelectedLineRange } from '@pierre/diffs' -import type { FileContents } from '@pierre/diffs' +import type { + FileContents, + FileDiffMetadata, + PostRenderPhase, + SelectedLineRange +} from '@pierre/diffs' import type { EditorOptions } from '@pierre/diffs/edit' import { useAppStore } from '@/store' import { RecoverableRenderErrorBoundary } from '@/components/error-boundaries/RecoverableRenderErrorBoundary' @@ -103,11 +107,15 @@ export function PierreDiffSurface({ }), enableGutterUtility: Boolean(onAddComment), onGutterUtilityClick: onAddComment - ? (range: SelectedLineRange) => + ? (range: SelectedLineRange) => { + if (range.side === 'deletions' || range.endSide === 'deletions') { + return + } onAddComment({ lineNumber: Math.max(range.start, range.end), startLine: range.start === range.end ? undefined : Math.min(range.start, range.end) }) + } : undefined, onPostRender: onPostRender ? (node: HTMLElement, _instance: unknown, phase: PostRenderPhase) => @@ -121,8 +129,8 @@ export function PierreDiffSurface({ [settings, editorFontZoomLevel] ) const lineAnnotations = useMemo( - () => buildPierreDiffCommentAnnotations(comments, pendingComment), - [comments, pendingComment] + () => buildPierreDiffCommentAnnotations(comments, filePath, pendingComment), + [comments, filePath, pendingComment] ) const editorOptions = useMemo>( () => ({ diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-cache-identity.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-cache-identity.ts new file mode 100644 index 00000000000..69ad33158ec --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-cache-identity.ts @@ -0,0 +1,36 @@ +type CachedIdentity = { original: string; modified: string; key: string } + +const identities = new Map() +const MAX_RETAINED_CHARACTERS = 4_000_000 +let retainedCharacters = 0 +let nextIdentity = 0 + +// Pierre uses cache keys as content equality, including across mounted surfaces. +export function getPierreDiffCacheIdentity( + scope: string, + original: string, + modified: string +): string { + const previous = identities.get(scope) + if (previous?.original === original && previous.modified === modified) { + identities.delete(scope) + identities.set(scope, previous) + return previous.key + } + if (previous) { + retainedCharacters -= previous.original.length + previous.modified.length + identities.delete(scope) + } + const key = `orca-diff:${++nextIdentity}` + identities.set(scope, { original, modified, key }) + retainedCharacters += original.length + modified.length + while (identities.size > 64 || retainedCharacters > MAX_RETAINED_CHARACTERS) { + const oldest = identities.entries().next().value + if (!oldest) { + break + } + retainedCharacters -= oldest[1].original.length + oldest[1].modified.length + identities.delete(oldest[0]) + } + return key +} diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-change-targets.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-change-targets.ts new file mode 100644 index 00000000000..c63a16fe9d6 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-change-targets.ts @@ -0,0 +1,28 @@ +import type { FileDiffMetadata } from '@pierre/diffs' + +export type PierreDiffChangeTarget = { lineNumber: number; side: 'additions' | 'deletions' } + +export function getPierreDiffChangeTargets( + diff: FileDiffMetadata | null +): PierreDiffChangeTarget[] { + const targets: PierreDiffChangeTarget[] = [] + for (const hunk of diff?.hunks ?? []) { + let additionLine = hunk.additionStart + let deletionLine = hunk.deletionStart + for (const block of hunk.hunkContent) { + if (block.type === 'context') { + additionLine += block.lines + deletionLine += block.lines + } else { + targets.push( + block.additions > 0 + ? { lineNumber: Math.max(1, additionLine), side: 'additions' } + : { lineNumber: Math.max(1, deletionLine), side: 'deletions' } + ) + additionLine += block.additions + deletionLine += block.deletions + } + } + } + return targets +} diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-comment-annotations.test.tsx b/src/renderer/src/components/editor/pierre-diff/pierre-diff-comment-annotations.test.tsx new file mode 100644 index 00000000000..ba7302e8536 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-comment-annotations.test.tsx @@ -0,0 +1,37 @@ +import { describe, expect, it, vi } from 'vitest' +import type { DecoratedDiffComment } from '../../diff-comments/decorated-diff-comment' +import { buildPierreDiffCommentAnnotations } from './pierre-diff-comment-annotations' + +vi.mock('../../diff-comments/DiffCommentCard', () => ({ DiffCommentCard: () => null })) +vi.mock('../../diff-comments/DiffCommentPopover', () => ({ DiffCommentPopover: () => null })) +vi.mock('../NotesSendMenu', () => ({ NotesSendMenu: () => null })) + +const note = (filePath: string): DecoratedDiffComment => ({ + id: filePath, + filePath, + worktreeId: 'review', + lineNumber: 2, + body: 'review comment', + createdAt: 1, + side: 'modified' +}) + +describe('Pierre review annotations', () => { + it('keeps comments for other PR files out of this file', () => { + const annotations = buildPierreDiffCommentAnnotations( + [note('one.ts'), note('two.ts')], + 'two.ts' + ) + expect(annotations).toHaveLength(1) + expect(annotations[0].metadata).toMatchObject({ + kind: 'comment', + comment: { filePath: 'two.ts' } + }) + }) + + it('retains the range of an unsaved draft alongside existing notes', () => { + expect( + buildPierreDiffCommentAnnotations([note('one.ts')], 'one.ts', { lineNumber: 5, startLine: 3 }) + ).toHaveLength(2) + }) +}) diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-comment-annotations.tsx b/src/renderer/src/components/editor/pierre-diff/pierre-diff-comment-annotations.tsx index 7be031b6623..21807637cea 100644 --- a/src/renderer/src/components/editor/pierre-diff/pierre-diff-comment-annotations.tsx +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-comment-annotations.tsx @@ -21,13 +21,16 @@ export type PierreDiffCommentAnnotation = DiffLineAnnotation ({ - side: 'additions', - lineNumber: comment.lineNumber, - metadata: { kind: 'comment', comment } - })) + const annotations: PierreDiffCommentAnnotation[] = comments + .filter((comment) => comment.filePath === filePath) + .map((comment) => ({ + side: 'additions', + lineNumber: comment.lineNumber, + metadata: { kind: 'comment', comment } + })) if (draft) { annotations.push({ side: 'additions', diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-context-copy.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-context-copy.ts index 49c4c7d5642..f8fdd219700 100644 --- a/src/renderer/src/components/editor/pierre-diff/pierre-diff-context-copy.ts +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-context-copy.ts @@ -2,6 +2,7 @@ import { formatCopiedSelectionWithContext } from '../selection-copy' import { editorShortcutMatches } from '../editor-shortcuts' import { formatShortcutLabel } from '@/hooks/useShortcutLabel' import { useAppStore } from '@/store' +import { getPierreSelectionRange } from './pierre-diff-selection' import { PRIMARY_SELECTION_MAX_LENGTH, isPrimarySelectionEnabled, @@ -10,15 +11,6 @@ import { const PRIMARY_SELECTION_DEBOUNCE_MS = 200 -/** Resolves the 1-based line of the row containing a selection boundary node. */ -function lineOf(node: Node | null): number | null { - const element = node instanceof Element ? node : (node?.parentElement ?? null) - const row = element?.closest('[data-line]') - const raw = row?.getAttribute('data-line') - const parsed = raw ? Number.parseInt(raw, 10) : Number.NaN - return Number.isFinite(parsed) ? parsed : null -} - /** * Restores `editor.copyContext` for Pierre-rendered diffs. Monaco exposed the * selection as an IRange; here the equivalent comes from the shadow root's @@ -32,14 +24,19 @@ export function installPierreContextualCopy( // is an overlay pinned to the selection's client rect instead. const hint = document.createElement('div') hint.className = - 'pointer-events-none fixed z-50 rounded-md border border-border/90 bg-background px-2.5 py-1 text-xs font-medium text-foreground shadow-[0_6px_18px_rgba(15,23,42,0.18)] backdrop-blur whitespace-nowrap' + 'pointer-events-none fixed z-50 rounded-md border border-border/90 bg-background px-2.5 py-1 text-xs font-medium text-foreground shadow-floating backdrop-blur whitespace-nowrap' hint.style.display = 'none' document.body.appendChild(hint) let primarySelectionTimer: number | null = null const readSelection = (): Selection | null => { const root = container.querySelector('diffs-container')?.shadowRoot - return (root as unknown as { getSelection?: () => Selection | null })?.getSelection?.() ?? null + const selection = + (root as unknown as { getSelection?: () => Selection | null })?.getSelection?.() ?? null + return root?.contains(selection?.anchorNode ?? null) && + root.contains(selection?.focusNode ?? null) + ? selection + : null } const hideHint = (): void => { @@ -49,10 +46,9 @@ export function installPierreContextualCopy( const updateHint = (): void => { const selection = readSelection() const text = selection?.toString() ?? '' - const startLine = lineOf(selection?.anchorNode ?? null) - const endLine = lineOf(selection?.focusNode ?? null) + const range = getPierreSelectionRange(selection) // Why: copy-with-context is a multi-line affordance; a single line copies plainly. - if (!text || startLine == null || endLine == null || startLine === endLine) { + if (!text || !range || range.startLineNumber === range.endLineNumber) { hideHint() return } @@ -95,19 +91,13 @@ export function installPierreContextualCopy( if (!editorShortcutMatches('editor.copyContext', event)) { return } - const host = container.querySelector('diffs-container') - const root = host?.shadowRoot - // Why: Chromium scopes the selection to the shadow root that owns the range. - const selection = ( - root as unknown as { getSelection?: () => Selection | null } - )?.getSelection?.() + const selection = readSelection() const selectedText = selection?.toString() ?? '' if (!selectedText) { return } - const startLine = lineOf(selection?.anchorNode ?? null) - const endLine = lineOf(selection?.focusNode ?? null) - if (startLine == null || endLine == null) { + const range = getPierreSelectionRange(selection) + if (!range) { return } const { relativePath, language } = getFileInfo() @@ -115,12 +105,7 @@ export function installPierreContextualCopy( relativePath, language, selectedText, - selection: { - startLineNumber: Math.min(startLine, endLine), - endLineNumber: Math.max(startLine, endLine), - startColumn: 1, - endColumn: 1 - } + selection: range }) if (!formatted) { return @@ -133,9 +118,11 @@ export function installPierreContextualCopy( container.addEventListener('keydown', handleKeyDown, true) document.addEventListener('selectionchange', handleSelectionChange) + document.addEventListener('scroll', hideHint, true) return () => { container.removeEventListener('keydown', handleKeyDown, true) document.removeEventListener('selectionchange', handleSelectionChange) + document.removeEventListener('scroll', hideHint, true) if (primarySelectionTimer !== null) { window.clearTimeout(primarySelectionTimer) } diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-metadata.test.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-metadata.test.ts new file mode 100644 index 00000000000..322eca3e236 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-metadata.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' +import { buildPierreFileDiff } from './pierre-diff-metadata' +import { getPierreDiffChangeTargets } from './pierre-diff-change-targets' + +const input = { + path: 'example.ts', + status: 'modified', + cacheKey: 'workspace:file:0', + originalContent: 'one\ntwo\nthree\nfour\nfive\n', + modifiedContent: 'one\nTWO\nthree\nFOUR\nfive\n', + parseDiffOptions: { ignoreWhitespace: true } +} + +describe('Pierre diff content identity and navigation', () => { + it('reuses unchanged content but invalidates edits, external updates, and whitespace options', () => { + const first = buildPierreFileDiff(input) + expect(buildPierreFileDiff(input).cacheKey).toBe(first.cacheKey) + for (const update of [ + { modifiedContent: 'new draft\n' }, + { originalContent: 'new base\n' }, + { parseDiffOptions: { ignoreWhitespace: false } }, + { path: 'another.ts' } + ]) { + expect(buildPierreFileDiff({ ...input, ...update }).cacheKey).not.toBe(first.cacheKey) + } + }) + + it('does not share stale content across workspaces with the same path and generation', () => { + const first = buildPierreFileDiff(input) + const second = buildPierreFileDiff({ ...input, modifiedContent: 'another workspace\n' }) + expect(second.cacheKey).not.toBe(first.cacheKey) + expect(second.additionLines.join('')).toBe('another workspace\n') + }) + + it.each(['added', 'untracked', 'deleted'])('gives %s files worker cache identities', (status) => { + expect(buildPierreFileDiff({ ...input, status }).cacheKey).toBeTruthy() + }) + + it('navigates every change inside one context hunk', () => { + const diff = buildPierreFileDiff(input) + expect(diff.hunks).toHaveLength(1) + expect(getPierreDiffChangeTargets(diff)).toEqual([ + { lineNumber: 2, side: 'additions' }, + { lineNumber: 4, side: 'additions' } + ]) + }) + + it('targets the old side for pure deletions, including deleted files', () => { + expect( + getPierreDiffChangeTargets( + buildPierreFileDiff({ + ...input, + modifiedContent: 'one\nthree\nfour\nfive\n' + }) + ) + ).toEqual([{ lineNumber: 2, side: 'deletions' }]) + expect( + getPierreDiffChangeTargets(buildPierreFileDiff({ ...input, status: 'deleted' })) + ).toEqual([{ lineNumber: 1, side: 'deletions' }]) + }) +}) diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-metadata.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-metadata.ts index 669fd4b31af..11686186715 100644 --- a/src/renderer/src/components/editor/pierre-diff/pierre-diff-metadata.ts +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-metadata.ts @@ -1,5 +1,6 @@ import { parseDiffFromFile, type FileContents, type FileDiffMetadata } from '@pierre/diffs' import type { CreatePatchOptionsNonabortable } from 'diff' +import { getPierreDiffCacheIdentity } from './pierre-diff-cache-identity' /** Statuses whose old side does not exist, so Pierre should render an add. */ const ADDED_STATUSES = new Set(['added', 'untracked']) @@ -36,12 +37,15 @@ export function buildPierreFileDiff({ }): FileDiffMetadata { const isAdded = ADDED_STATUSES.has(status) const isDeleted = status === 'deleted' + const identity = getPierreDiffCacheIdentity( + JSON.stringify([cacheKey, path, oldPath, status, parseDiffOptions]), + originalContent, + modifiedContent + ) const oldFile = isAdded ? null - : toFileContents(oldPath ?? path, originalContent, cacheKey && `${cacheKey}:old`) - const newFile = isDeleted - ? null - : toFileContents(path, modifiedContent, cacheKey && `${cacheKey}:new`) + : toFileContents(oldPath ?? path, originalContent, `${identity}:old`) + const newFile = isDeleted ? null : toFileContents(path, modifiedContent, `${identity}:new`) - return parseDiffFromFile(oldFile, newFile, parseDiffOptions) + return { ...parseDiffFromFile(oldFile, newFile, { ...parseDiffOptions }), cacheKey: identity } } diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-render-gate.test.tsx b/src/renderer/src/components/editor/pierre-diff/pierre-diff-render-gate.test.tsx new file mode 100644 index 00000000000..cd15401a0b3 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-render-gate.test.tsx @@ -0,0 +1,65 @@ +// @vitest-environment happy-dom +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import DiffViewer from '../DiffViewer' +import { buildPierreFileDiff } from './pierre-diff-metadata' +import { getLargeDiffRenderLimit } from '../large-diff-render-limit' + +vi.mock('@/store', () => ({ useAppStore: (selector: (s: object) => unknown) => selector({}) })) +vi.mock('../editor-shortcuts', () => ({ installEditorSaveShortcut: () => () => {} })) +vi.mock('../diff-navigation-context', () => ({ + useDiffNavigatorRegistration: () => ({ + registerDiffNavigator: () => {}, + unregisterDiffNavigator: () => {} + }) +})) +vi.mock('./pierre-diff-metadata', () => ({ + buildPierreFileDiff: vi.fn(() => { + throw new Error('limited content reached parser') + }) +})) +vi.mock('./PierreDiffProviders', () => ({ PierreDiffProviders: () => null })) +vi.mock('./PierreDiffSurface', () => ({ PierreDiffSurface: () => null })) +vi.mock('../LargeDiffFallback', () => ({ LargeDiffFallback: () =>
Large diff fallback
})) + +afterEach(() => { + cleanup() + vi.clearAllMocks() +}) + +it('renders a large-file fallback without synchronously computing the diff', () => { + const modifiedContent = 'x'.repeat(6_000_001) + render( + + ) + expect(screen.getByText('Large diff fallback')).toBeTruthy() + expect(buildPierreFileDiff).not.toHaveBeenCalled() +}) + +it('honors a host-provided render limit when the RPC bodies are omitted', () => { + render( + + ) + expect(screen.getByText('Large diff fallback')).toBeTruthy() + expect(buildPierreFileDiff).not.toHaveBeenCalled() +}) diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-scroll.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-scroll.ts index 164187d0270..c536d5dc007 100644 --- a/src/renderer/src/components/editor/pierre-diff/pierre-diff-scroll.ts +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-scroll.ts @@ -7,19 +7,28 @@ export function scrollPierreDiffToLine({ host, container, lineNumber, + side = 'additions', hunkIndex, hunkCount }: { host: HTMLElement | null container: HTMLElement | null lineNumber: number + side?: 'additions' | 'deletions' hunkIndex: number hunkCount: number }): boolean { if (!container) { return false } - const row = host?.shadowRoot?.querySelector(`[data-line="${lineNumber}"]`) + const lineType = side === 'additions' ? 'change-addition' : 'change-deletion' + const root = host?.shadowRoot + const row = + root?.querySelector(`[data-code][data-${side}] [data-line="${lineNumber}"]`) ?? + root?.querySelector(`[data-code] [data-line="${lineNumber}"][data-line-type="${lineType}"]`) ?? + root?.querySelector( + `[data-code]:not([data-deletions]) [data-line="${lineNumber}"]:not([data-line-type="change-deletion"])` + ) if (row instanceof HTMLElement) { const offset = row.getBoundingClientRect().top - container.getBoundingClientRect().top container.scrollTop += offset - container.clientHeight / 3 diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-selection.test.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-selection.test.ts new file mode 100644 index 00000000000..bbd34cf5f0d --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-selection.test.ts @@ -0,0 +1,45 @@ +// @vitest-environment happy-dom +import { describe, expect, it } from 'vitest' +import { getPierreSelectionRange } from './pierre-diff-selection' +import { formatCopiedSelectionWithContext } from '../selection-copy' + +function selection(endOffset: number): Selection { + const root = document.createElement('div') + root.innerHTML = + '
first line
last line
' + const range = document.createRange() + range.setStart(root.children[0].firstChild!.firstChild!, 1) + range.setEnd(root.children[1].firstChild!.firstChild!, endOffset) + return { rangeCount: 1, getRangeAt: () => range } as unknown as Selection +} + +describe('Pierre contextual copy boundaries', () => { + it('includes a partially selected final line in the label', () => { + const range = getPierreSelectionRange(selection(2))! + expect(range).toEqual({ startLineNumber: 10, startColumn: 2, endLineNumber: 11, endColumn: 3 }) + expect( + formatCopiedSelectionWithContext({ + relativePath: 'file.ts', + language: 'typescript', + selectedText: 'irst line\nla', + selection: range + }) + ).toContain('Lines: 10-11') + }) + + it('excludes a final line selected only at its first column', () => { + const range = getPierreSelectionRange(selection(0))! + expect( + formatCopiedSelectionWithContext({ + relativePath: 'file.ts', + language: 'typescript', + selectedText: 'irst line\n', + selection: range + }) + ).toContain('Line: 10\n') + }) + + it('ignores absent selections', () => { + expect(getPierreSelectionRange(null)).toBeNull() + }) +}) diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-selection.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-selection.ts new file mode 100644 index 00000000000..e197d6315d9 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-selection.ts @@ -0,0 +1,33 @@ +import type { IRange } from 'monaco-editor' + +function selectionBoundary(node: Node, offset: number): { line: number; column: number } | null { + const element = node instanceof Element ? node : node.parentElement + const row = element?.closest('[data-line]') + const line = Number(row?.getAttribute('data-line')) + if (!row || !Number.isInteger(line) || line < 1) { + return null + } + const prefix = document.createRange() + prefix.selectNodeContents(row) + prefix.setEnd(node, offset) + return { line, column: prefix.toString().length + 1 } +} + +// DOM ranges are ordered even when the user drags backwards. +export function getPierreSelectionRange(selection: Selection | null): IRange | null { + if (!selection?.rangeCount) { + return null + } + const range = selection.getRangeAt(0) + const start = selectionBoundary(range.startContainer, range.startOffset) + const end = selectionBoundary(range.endContainer, range.endOffset) + if (!start || !end) { + return null + } + return { + startLineNumber: start.line, + startColumn: start.column, + endLineNumber: end.line, + endColumn: end.column + } +} diff --git a/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-find.test.tsx b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-find.test.tsx new file mode 100644 index 00000000000..3739c3e0cf7 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-find.test.tsx @@ -0,0 +1,72 @@ +// @vitest-environment happy-dom +import { act, renderHook, cleanup } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { usePierreDiffFind } from './use-pierre-diff-find' + +vi.mock('../editor-shortcuts', () => ({ + editorShortcutMatches: (_: string, event: KeyboardEvent) => event.key === 'f' && event.ctrlKey +})) +vi.mock('@/lib/shortcut-platform', () => ({ getShortcutPlatform: () => 'linux' })) + +afterEach(() => { + cleanup() + vi.useRealTimers() + document.body.replaceChildren() +}) + +function setup(isEditable: boolean) { + vi.useFakeTimers() + const container = document.createElement('div') + const host = document.createElement('diffs-container') + const shadow = host.attachShadow({ mode: 'open' }) + container.append(host) + document.body.append(container) + const { result } = renderHook(() => + usePierreDiffFind({ isEditable, containerRef: { current: container } }) + ) + const attachContent = () => { + const content = document.createElement('div') + content.setAttribute('contenteditable', 'true') + shadow.append(content) + return content + } + const find = () => + act(() => + result.current.handleContainerKeyDown({ + key: 'f', + ctrlKey: true, + preventDefault: vi.fn(), + stopPropagation: vi.fn() + } as unknown as React.KeyboardEvent) + ) + return { result, attachContent, find } +} + +describe('Pierre find shortcut', () => { + it('opens on the first press when the editable surface is already attached', () => { + const { attachContent, find } = setup(true) + const content = attachContent() + const search = vi.fn() + content.addEventListener('keydown', search) + find() + act(() => vi.runOnlyPendingTimers()) + expect(search).toHaveBeenCalledOnce() + expect(search.mock.calls[0][0]).toMatchObject({ key: 'f', ctrlKey: true }) + }) + + it('opens after a read-only find session attaches and cancels on Escape', () => { + const { result, attachContent, find } = setup(false) + find() + expect(result.current.editEnabled).toBe(true) + const content = attachContent() + const search = vi.fn() + content.addEventListener('keydown', search) + act(() => result.current.handleEditorAttach({ focus: vi.fn() })) + act(() => + result.current.handleContainerKeyDown({ key: 'Escape' } as React.KeyboardEvent) + ) + act(() => vi.runOnlyPendingTimers()) + expect(search).not.toHaveBeenCalled() + expect(result.current.editEnabled).toBe(false) + }) +}) diff --git a/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-find.ts b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-find.ts index efbc4d9b22b..443e4d13070 100644 --- a/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-find.ts +++ b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-find.ts @@ -1,4 +1,4 @@ -import { useCallback, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import type { EditorFocusOptions } from '@pierre/diffs/edit' import { getShortcutPlatform } from '@/lib/shortcut-platform' import { editorShortcutMatches } from '../editor-shortcuts' @@ -10,7 +10,7 @@ import { editorShortcutMatches } from '../editor-shortcuts' */ function findPierreContentElement(container: HTMLElement | null): HTMLElement | null { const host = container?.querySelector('diffs-container') - const editable = host?.shadowRoot?.querySelector('[contenteditable]') + const editable = host?.shadowRoot?.querySelector('[contenteditable="true"]') return editable instanceof HTMLElement ? editable : null } @@ -61,9 +61,44 @@ export function usePierreDiffFind({ }): PierreDiffFind { const [findActive, setFindActive] = useState(false) const pendingFindRef = useRef(false) + const replayingFindRef = useRef(false) + const findFrameRef = useRef(null) + + const openMountedSearch = useCallback(() => { + if (findFrameRef.current !== null) { + cancelAnimationFrame(findFrameRef.current) + } + findFrameRef.current = requestAnimationFrame(() => { + findFrameRef.current = null + const target = findPierreContentElement(containerRef.current) + if (!target || !pendingFindRef.current) { + return + } + pendingFindRef.current = false + target.focus({ preventScroll: true }) + replayingFindRef.current = true + try { + dispatchPierreOpenSearchPanel(target) + } finally { + replayingFindRef.current = false + } + }) + }, [containerRef]) + + useEffect( + () => () => { + if (findFrameRef.current !== null) { + cancelAnimationFrame(findFrameRef.current) + } + }, + [] + ) const handleContainerKeyDown = useCallback( (event: React.KeyboardEvent) => { + if (replayingFindRef.current) { + return + } // Why: a find-only session must not outlive the search panel, or a // read-only diff stays editable forever after a single Cmd+F. if (findActive && event.key === 'Escape') { @@ -71,15 +106,19 @@ export function usePierreDiffFind({ setFindActive(false) return } - if (findActive || !editorShortcutMatches('editor.find', event)) { + if (!editorShortcutMatches('editor.find', event)) { return } event.preventDefault() event.stopPropagation() pendingFindRef.current = true setFindActive(true) + // Editable surfaces already attached; toggling find does not reattach them. + if (findPierreContentElement(containerRef.current)) { + openMountedSearch() + } }, - [findActive] + [findActive, containerRef, openMountedSearch] ) // Why: leaving the surface ends a find-only session too; the panel is gone. @@ -99,20 +138,13 @@ export function usePierreDiffFind({ if (!pendingFindRef.current) { return } - pendingFindRef.current = false editor.focus({ lineNumber: 'first-visible', preventScroll: true }) // Why: the editable DOM is not focusable until after this commit paints, // so a same-tick dispatch misses Pierre's content element and the first // Cmd+F is swallowed — which is why it used to take two presses. - requestAnimationFrame(() => { - const target = findPierreContentElement(containerRef.current) - if (target) { - target.focus() - dispatchPierreOpenSearchPanel(target) - } - }) + openMountedSearch() }, - [containerRef] + [openMountedSearch] ) const exitFind = useCallback(() => {