diff --git a/src/renderer/src/components/editor/DiffViewer.mount-focus.test.tsx b/src/renderer/src/components/editor/DiffViewer.mount-focus.test.tsx new file mode 100644 index 00000000000..242a26b1e7f --- /dev/null +++ b/src/renderer/src/components/editor/DiffViewer.mount-focus.test.tsx @@ -0,0 +1,61 @@ +// @vitest-environment happy-dom +import { cleanup, render } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import DiffViewer from './DiffViewer' + +let latestAutoFocusHost: boolean | undefined + +vi.mock('@/store', () => ({ + useAppStore: (selector: (s: object) => unknown) => + selector({ + settings: {}, + addDiffComment: () => {}, + deleteDiffComment: () => {}, + updateDiffComment: () => {} + }) +})) +vi.mock('./editor-shortcuts', () => ({ installEditorSaveShortcut: () => () => {} })) +vi.mock('./diff-navigation-context', () => ({ + useDiffNavigatorRegistration: () => ({ + registerDiffNavigator: () => {}, + unregisterDiffNavigator: () => {} + }) +})) +vi.mock('./pierre-diff/use-pierre-file-diff', () => ({ + usePierreFileDiff: () => ({ + fileDiff: { name: 'file.ts', hunks: [] }, + error: null, + retry: () => {}, + markEdited: () => {}, + editReady: true + }) +})) +vi.mock('./pierre-diff/PierreDiffProviders', () => ({ + PierreDiffProviders: ({ children }: { children: React.ReactNode }) => children +})) +vi.mock('./pierre-diff/PierreDiffSurface', () => ({ + PierreDiffSurface: (props: { autoFocusHost?: boolean }) => { + latestAutoFocusHost = props.autoFocusHost + return null + } +})) + +afterEach(() => { + cleanup() + latestAutoFocusHost = undefined +}) + +it('asks the Pierre host to take focus the way Monaco did on DiffViewer mount', () => { + render( + + ) + expect(latestAutoFocusHost).toBe(true) +}) diff --git a/src/renderer/src/components/editor/DiffViewer.tsx b/src/renderer/src/components/editor/DiffViewer.tsx index eeaaa4ea886..fd2803c2408 100644 --- a/src/renderer/src/components/editor/DiffViewer.tsx +++ b/src/renderer/src/components/editor/DiffViewer.tsx @@ -118,6 +118,7 @@ export default function DiffViewer({ return } const navigator: DiffNavigator = { + id: modelKey, changeLines, container, scrollToChange: ({ lineNumber, hunkIndex, hunkCount }) => { @@ -140,6 +141,7 @@ export default function DiffViewer({ }, [ changeLines, changeTargets, + modelKey, registerDiffNavigator, renderLimit.limited, unregisterDiffNavigator @@ -275,6 +277,7 @@ export default function DiffViewer({ isEditable={Boolean(editable) && editReady} editStateKey={modelKey} collapseUnchanged={false} + autoFocusHost worktreeId={worktreeId ?? ''} filePath={relativePath} language={language} diff --git a/src/renderer/src/components/editor/diff-navigation-context.test.tsx b/src/renderer/src/components/editor/diff-navigation-context.test.tsx index cb64406a2a1..d6f93bd0163 100644 --- a/src/renderer/src/components/editor/diff-navigation-context.test.tsx +++ b/src/renderer/src/components/editor/diff-navigation-context.test.tsx @@ -15,8 +15,9 @@ type FakeNavigator = DiffNavigator & { scrollToChange: DiffNavigator['scrollToChange'] & { mock: unknown } } -function createFakeNavigator(changeLines: number[]): FakeNavigator { +function createFakeNavigator(changeLines: number[], id = 'file'): FakeNavigator { return { + id, changeLines, container: document.createElement('div'), scrollToChange: vi.fn() @@ -147,6 +148,45 @@ describe('DiffNavigationProvider', () => { expect(registrationRenderCount).toBe(1) }) + it('keeps the F7 cursor across a same-file unregister/re-register (re-parse)', () => { + mount() + const first = createFakeNavigator([4, 20, 61, 80], 'a.ts') + act(() => registration?.registerDiffNavigator(first)) + act(() => captured?.goToNextDiff()) + act(() => captured?.goToNextDiff()) + act(() => captured?.goToNextDiff()) + + const parsed = createFakeNavigator([4, 20, 61, 80], 'a.ts') + act(() => registration?.unregisterDiffNavigator(first)) + act(() => registration?.registerDiffNavigator(parsed)) + act(() => captured?.goToNextDiff()) + + expect(parsed.scrollToChange).toHaveBeenLastCalledWith({ + lineNumber: 80, + hunkIndex: 3, + hunkCount: 4 + }) + }) + + it('resets the F7 cursor when a different file registers', () => { + mount() + const first = createFakeNavigator([4, 20, 61, 80], 'a.ts') + act(() => registration?.registerDiffNavigator(first)) + act(() => captured?.goToNextDiff()) + act(() => captured?.goToNextDiff()) + act(() => captured?.goToNextDiff()) + + const nextFile = createFakeNavigator([1, 2, 3, 4], 'b.ts') + act(() => registration?.registerDiffNavigator(nextFile)) + act(() => captured?.goToNextDiff()) + + expect(nextFile.scrollToChange).toHaveBeenCalledWith({ + lineNumber: 1, + hunkIndex: 0, + hunkCount: 4 + }) + }) + it('installs a capture-phase key listener on register and removes it on unregister', () => { mount() const navigator = createFakeNavigator([2]) diff --git a/src/renderer/src/components/editor/diff-navigation-context.tsx b/src/renderer/src/components/editor/diff-navigation-context.tsx index 9e98d76e6a5..f848bed50d4 100644 --- a/src/renderer/src/components/editor/diff-navigation-context.tsx +++ b/src/renderer/src/components/editor/diff-navigation-context.tsx @@ -3,6 +3,8 @@ import { installDiffChangeNavigationShortcut } from './editor-shortcuts' /** A mounted diff that can report its changes and scroll to one. */ export type DiffNavigator = { + /** File identity. Re-parses keep this so F7's cursor is not reset. */ + id: string /** Modified-side start line of every hunk, in document order. */ changeLines: readonly number[] scrollToChange: (args: { lineNumber: number; hunkIndex: number; hunkCount: number }) => void @@ -46,6 +48,9 @@ export function DiffNavigationProvider({ // Why: the cursor is provider-owned because the renderer no longer tracks a // "current change" of its own the way Monaco's goToDiff did. const cursorRef = useRef(-1) + // Why: React effect re-registers by unregistering first, so identity must + // outlive navigatorRef or a same-file re-parse would look like a new file. + const fileIdRef = useRef(null) // Why: changeCount must be state, not a ref — the header is a sibling consumer // and only re-renders (enabling the buttons) when the value identity changes. const [changeCount, setChangeCount] = useState(0) @@ -78,8 +83,16 @@ export function DiffNavigationProvider({ const registerDiffNavigator = useCallback( (navigator: DiffNavigator) => { + if (fileIdRef.current !== navigator.id) { + cursorRef.current = -1 + } else { + const total = navigator.changeLines.length + if (cursorRef.current >= total) { + cursorRef.current = total === 0 ? -1 : total - 1 + } + } + fileIdRef.current = navigator.id navigatorRef.current = navigator - cursorRef.current = -1 // Hold at most one keyboard listener; replace any prior navigator's. shortcutCleanupRef.current?.() shortcutCleanupRef.current = installDiffChangeNavigationShortcut( @@ -100,7 +113,6 @@ export function DiffNavigationProvider({ shortcutCleanupRef.current?.() shortcutCleanupRef.current = null navigatorRef.current = null - cursorRef.current = -1 setChangeCount(0) }, []) diff --git a/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.mount-focus.test.tsx b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.mount-focus.test.tsx new file mode 100644 index 00000000000..9b243ca3d72 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.mount-focus.test.tsx @@ -0,0 +1,82 @@ +// @vitest-environment happy-dom +import { cleanup, render } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import type { FileDiffMetadata } from '@pierre/diffs' +import { PierreDiffSurface } from './PierreDiffSurface' + +vi.mock('@pierre/diffs/react', () => ({ FileDiff: () => null })) +vi.mock('./pierre-diff-context-copy', () => ({ installPierreContextualCopy: () => () => {} })) +vi.mock('./use-pierre-diff-find', () => ({ + usePierreDiffFind: () => ({ + searchBar: null, + handleContainerKeyDown: () => {}, + onPointerDown: () => {}, + onPostRender: () => {}, + onEditChange: () => {} + }) +})) +vi.mock('./use-pierre-diff-note-navigation', () => ({ + usePierreDiffNoteNavigation: () => () => {} +})) +vi.mock('./use-pierre-diff-shift-wheel', () => ({ + usePierreDiffShiftWheel: () => () => {} +})) +vi.mock('./use-pierre-diff-native-view', () => ({ + usePierreDiffNativeView: () => () => {} +})) +vi.mock('@/store', () => { + const state = { + editorFontZoomLevel: 0, + clearDeliveredDiffComments: () => {}, + activeGroupIdByWorktree: {}, + scrollToDiffCommentId: null, + keybindings: {} + } + const useAppStore = (selector: (s: typeof state) => unknown) => selector(state) + useAppStore.getState = () => state + return { useAppStore } +}) + +const fileDiff = { name: 'file.ts', hunks: [] } as unknown as FileDiffMetadata + +function renderSurface(autoFocusHost?: boolean) { + return render( + {}} + /> + ) +} + +afterEach(() => { + cleanup() + document.body.replaceChildren() +}) + +it('focuses the keyboard host on mount for the single-file tab', () => { + const sidebar = document.createElement('button') + document.body.append(sidebar) + sidebar.focus() + expect(document.activeElement).toBe(sidebar) + + const { container } = renderSurface(true) + const host = container.querySelector('[data-editor-keyboard-scope]') + expect(host).not.toBeNull() + expect(document.activeElement).toBe(host) +}) + +it('does not steal focus on mount for combined-diff rows', () => { + const sidebar = document.createElement('button') + document.body.append(sidebar) + sidebar.focus() + + renderSurface() + expect(document.activeElement).toBe(sidebar) +}) diff --git a/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.test.tsx b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.test.tsx new file mode 100644 index 00000000000..a86e474a38d --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.test.tsx @@ -0,0 +1,127 @@ +// @vitest-environment happy-dom +import { cleanup, render } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import type { FileDiffMetadata, PostRenderPhase } from '@pierre/diffs' +import type { PierreDiffInstance } from './PierreDiffSurface' +import { PierreDiffSurface } from './PierreDiffSurface' + +const captured = vi.hoisted(() => ({ + options: [] as { onPostRender?: (...args: unknown[]) => void }[] +})) +const consumers = vi.hoisted(() => ({ + height: vi.fn(), + note: vi.fn(), + search: vi.fn(), + wheel: vi.fn(), + native: vi.fn() +})) + +vi.mock('@pierre/diffs/react', () => ({ + FileDiff: (props: { options: { onPostRender?: (...args: unknown[]) => void } }) => { + captured.options.push(props.options) + return
+ } +})) +vi.mock('@/store', () => ({ + useAppStore: (selector: (state: object) => unknown) => + selector({ + editorFontZoomLevel: 0, + clearDeliveredDiffComments: vi.fn(), + activeGroupIdByWorktree: {} + }) +})) +vi.mock('@/components/error-boundaries/RecoverableRenderErrorBoundary', () => ({ + RecoverableRenderErrorBoundary: ({ children }: { children: React.ReactNode }) => children +})) +vi.mock('./use-pierre-diff-find', () => ({ + usePierreDiffFind: () => ({ + searchBar: null, + handleContainerKeyDown: () => {}, + onPointerDown: () => {}, + onPostRender: (...args: unknown[]) => consumers.search(...args), + onEditChange: () => {} + }) +})) +vi.mock('./use-pierre-diff-note-navigation', () => ({ + usePierreDiffNoteNavigation: () => consumers.note +})) +vi.mock('./use-pierre-diff-shift-wheel', () => ({ + usePierreDiffShiftWheel: () => consumers.wheel +})) +vi.mock('./use-pierre-diff-native-view', () => ({ + usePierreDiffNativeView: () => consumers.native +})) +vi.mock('./pierre-diff-context-copy', () => ({ + installPierreContextualCopy: () => () => {} +})) + +afterEach(() => { + cleanup() + captured.options.length = 0 + vi.clearAllMocks() +}) + +function renderSurface(onPostRender = consumers.height) { + return render( + {}} + onPostRender={onPostRender} + /> + ) +} + +it('keeps Pierre onPostRender stable across search-only rerenders and still chains consumers', () => { + const view = renderSurface() + const first = captured.options.at(-1)?.onPostRender + expect(first).toEqual(expect.any(Function)) + const node = document.createElement('div') + const instance = {} as PierreDiffInstance + first?.(node, instance, 'mount' as PostRenderPhase) + expect(consumers.height).toHaveBeenCalledWith(node, 'mount', instance) + expect(consumers.note).toHaveBeenCalledWith(node, 'mount', instance) + expect(consumers.search).toHaveBeenCalledWith(node, 'mount', instance) + expect(consumers.wheel).toHaveBeenCalledWith(node, 'mount', instance) + expect(consumers.native).toHaveBeenCalledWith(node, 'mount', instance) + view.rerender( + {}} + onPostRender={vi.fn()} + /> + ) + expect(captured.options.at(-1)?.onPostRender).toBe(first) + const latestHeight = vi.fn() + view.rerender( + {}} + onPostRender={latestHeight} + /> + ) + expect(captured.options.at(-1)?.onPostRender).toBe(first) + first?.(node, instance, 'update' as PostRenderPhase) + expect(latestHeight).toHaveBeenCalledWith(node, 'update', instance) + expect(consumers.note).toHaveBeenCalledWith(node, 'update', instance) + expect(consumers.search).toHaveBeenCalledWith(node, 'update', instance) + expect(consumers.wheel).toHaveBeenCalledWith(node, 'update', instance) + expect(consumers.native).toHaveBeenCalledWith(node, 'update', instance) +}) diff --git a/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx index 648694e6ef7..dc41d410e6b 100644 --- a/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx +++ b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx @@ -47,6 +47,8 @@ export type PierreDiffSurfaceProps = { editStateKey?: string /** Collapse unchanged context. Combined diffs do; the single-file tab does not. */ collapseUnchanged: boolean + /** Single-file tab only. Combined rows must not steal focus on mount. */ + autoFocusHost?: boolean worktreeId: string filePath: string comments: readonly DecoratedDiffComment[] @@ -82,6 +84,7 @@ export function PierreDiffSurface({ isEditable, editStateKey, collapseUnchanged, + autoFocusHost = false, worktreeId, filePath, comments, @@ -107,6 +110,14 @@ export function PierreDiffSurface({ ) const containerRef = useRef(null) const editorRef = useRef | null>(null) + // Why: Monaco focused the single-file DiffEditor on mount so Cmd+F/F7 worked + // without a click. Combined DiffSectionItem did not — do not steal there. + useLayoutEffect(() => { + if (!autoFocusHost) { + return + } + containerRef.current?.focus({ preventScroll: true }) + }, [autoFocusHost]) const onEditChangeRef = useRef(onEditChange) const { searchBar, @@ -131,6 +142,31 @@ export function PierreDiffSurface({ editorRef ) const navigateToNote = usePierreDiffNoteNavigation({ worktreeId, filePath, comments }) + const postRenderRef = useRef({ + onPostRender, + navigateToNote, + searchPostRender, + shiftWheelPostRender, + nativeViewPostRender + }) + postRenderRef.current = { + onPostRender, + navigateToNote, + searchPostRender, + shiftWheelPostRender, + nativeViewPostRender + } + const handlePostRender = useCallback( + (node: HTMLElement, instance: PierreDiffInstance, phase: PostRenderPhase) => { + const chain = postRenderRef.current + chain.onPostRender?.(node, phase, instance) + chain.navigateToNote(node, phase, instance) + chain.searchPostRender(node, phase, instance) + chain.shiftWheelPostRender(node, phase, instance) + chain.nativeViewPostRender(node, phase, instance) + }, + [] + ) const commentableLines = useMemo( () => (commentableLineNumbers ? new Set(commentableLineNumbers) : null), [commentableLineNumbers] @@ -173,24 +209,14 @@ export function PierreDiffSurface({ }) } : undefined, - onPostRender: (node: HTMLElement, instance: PierreDiffInstance, phase: PostRenderPhase) => { - onPostRender?.(node, phase, instance) - navigateToNote(node, phase, instance) - searchPostRender(node, phase, instance) - shiftWheelPostRender(node, phase, instance) - nativeViewPostRender(node, phase, instance) - } + onPostRender: handlePostRender }), [ settings, sideBySide, collapseUnchanged, - onPostRender, onAddComment, - navigateToNote, - searchPostRender, - shiftWheelPostRender, - nativeViewPostRender, + handlePostRender, commentableLines, addCommentLabel ] @@ -302,9 +328,11 @@ export function PierreDiffSurface({ boundaryId="editor.pierre-diff-surface" surface="page" compact - // Why: include the remount identity, not just the name — a caught render throw - // otherwise stays latched until the row unmounts, even after content changes. - resetKey={`${editStateKey ?? fileDiff.name}:${fileDiff.name}`} + // Why cacheKey and not just the name: editStateKey is tab/section identity and name is the + // path, so neither moves when only the contents change. A caught render throw would stay + // latched through a save, an agent rewrite or a refetch, because the same surface stays + // mounted while `sameFile` holds. cacheKey is derived from the content. + resetKey={`${editStateKey ?? fileDiff.name}:${fileDiff.cacheKey ?? fileDiff.name}`} title={translate('editor.diff.renderFailed', 'This diff could not be rendered')} description={translate( 'editor.diff.renderRetry', diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-edit-state.test.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-edit-state.test.ts index cf13573db37..6d7b27f9fe6 100644 --- a/src/renderer/src/components/editor/pierre-diff/pierre-diff-edit-state.test.ts +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-edit-state.test.ts @@ -26,6 +26,7 @@ vi.mock('@pierre/diffs/edit', () => ({ edit() { return () => this.options.onComplete?.({}) } + cleanUp(_reason?: 'discard' | 'recycle' | 'complete') {} } })) @@ -109,6 +110,39 @@ it('keeps simultaneous copies of a scope independent', () => { reopened.finish() }) +it('releases the scope when cleanup finishes without emitting complete', () => { + const editor = createPierreEditor( + 'file-diff', + withPierreDiffEditState({}, 'orphaned', { + type: 'change', + deletionLines: ['old\n'], + additionLines: ['new\n'] + } as FileDiffMetadata) + ) + editor.edit({} as never) + editor.cleanUp('discard') + const reopened = create('orphaned') + expect(reopened.key).toBe('orphaned') + reopened.finish() +}) + +it('keeps the scope reserved across a recycle so a remount cannot share the key', () => { + const editor = createPierreEditor( + 'file-diff', + withPierreDiffEditState({}, 'recycled', { + type: 'change', + deletionLines: ['old\n'], + additionLines: ['new\n'] + } as FileDiffMetadata) + ) + editor.edit({} as never) + editor.cleanUp('recycle') + const concurrent = create('recycled') + expect(concurrent.key).not.toBe('recycled') + concurrent.finish() + editor.cleanUp('discard') +}) + it('evicts dormant document history when retained text exceeds the budget', () => { const editor = create('oversized-history') const state = stored() diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-edit-state.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-edit-state.ts index 202a2c7be37..a8039638d09 100644 --- a/src/renderer/src/components/editor/pierre-diff/pierre-diff-edit-state.ts +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-edit-state.ts @@ -96,6 +96,17 @@ export const createPierreEditor: EditorFactory { + if (!active.has(key)) { + return + } + active.delete(key) + if (duplicate) { + EditStateManager.clear('file-diff', key) + } else { + retainDormant(key) + } + } try { const editor = new Editor( editorType, @@ -103,18 +114,14 @@ export const createPierreEditor: EditorFactory { - active.delete(key) - if (duplicate) { - EditStateManager.clear('file-diff', key) - } else { - retainDormant(key) - } + release() options.onComplete?.(event) } }, key ) const edit = editor.edit.bind(editor) + const cleanUp = editor.cleanUp.bind(editor) editor.edit = (instance) => { try { return edit(instance) @@ -123,6 +130,17 @@ export const createPierreEditor: EditorFactory { + try { + cleanUp(reason) + } finally { + if (reason !== 'recycle') { + release() + } + } + } return editor } catch (error) { active.delete(key) diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.test.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.test.ts new file mode 100644 index 00000000000..b08c7bfbc3f --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.test.ts @@ -0,0 +1,35 @@ +import { expect, it } from 'vitest' +import { pierreSearchRevealLine } from './pierre-diff-search-view' +import { buildPierreFileDiff } from './pierre-diff-metadata' + +const input = { + path: 'file.ts', + status: 'modified', + cacheKey: 'search-view', + parseDiffOptions: {} +} + +it('passes addition-side matches through unchanged', () => { + const diff = buildPierreFileDiff({ + ...input, + originalContent: 'a\n', + modifiedContent: 'b\n' + }) + expect(pierreSearchRevealLine(diff, 1, 'additions')).toBe(1) +}) + +it('does not feed a pure-deletion line number to revealLine', () => { + const diff = buildPierreFileDiff({ + ...input, + originalContent: `${'keep\n'.repeat(40)}${'drop\n'.repeat(10)}GONE\n${'keep\n'.repeat(40)}old\n`, + modifiedContent: `${'keep\n'.repeat(80)}new\n` + }) + const goneLine = 51 + const hunk = diff.hunks[0] + const additionEnd = hunk.additionStart + hunk.additionCount + expect(goneLine).toBeGreaterThan(additionEnd) + const revealed = pierreSearchRevealLine(diff, goneLine, 'deletions') + expect(revealed).not.toBe(goneLine) + expect(revealed).toBeGreaterThanOrEqual(hunk.additionStart) + expect(revealed).toBeLessThanOrEqual(additionEnd) +}) diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.ts index 0a037529592..25379425227 100644 --- a/src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.ts +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.ts @@ -146,17 +146,21 @@ export function pierreSearchRevealLine( if (side === 'additions') { return lineNumber } - let modifiedLine = lineNumber + // revealLine is new-file only; never pass a pure-deletion old-file number. + let modifiedLine = 0 + let found = false iterateOverDiff({ diff, diffStyle: 'unified', expandedHunks: true, - callback: ({ deletionLine, additionLine }) => { - if (deletionLine?.lineNumber !== lineNumber) { - return + callback: ({ deletionLine, additionLine }): boolean => { + if (additionLine) { + modifiedLine = additionLine.lineNumber } - modifiedLine = additionLine?.lineNumber ?? lineNumber - return true + if (deletionLine?.lineNumber === lineNumber) { + found = true + } + return found && additionLine != null } }) return modifiedLine 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 index e2e6912597f..b711403d9ff 100644 --- 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 @@ -76,6 +76,27 @@ it('searches original content and restores its native selection on close', () => expect(editor.setDeletedTextSelectionActive).toHaveBeenCalledWith(true) }) +it('keeps replace targeting additions after Cmd+H then Cmd+F', () => { + results.mockReturnValue(null) + const { result, find } = setup(true) + const deleted = document.createElement('div') + deleted.setAttribute('data-code', '') + deleted.setAttribute('data-deletions', '') + act(() => + result.current.onPointerDown({ + nativeEvent: { composedPath: () => [deleted] } + } as unknown as React.PointerEvent) + ) + find('h') + expect(result.current.searchBar?.side).toBe('additions') + expect(result.current.searchBar?.canReplace).toBe(true) + expect(result.current.searchBar?.replaceOpen).toBe(true) + find('f') + expect(result.current.searchBar?.side).toBe('additions') + expect(result.current.searchBar?.canReplace).toBe(true) + expect(results.mock.lastCall?.[0].text).toBe('modified') +}) + it('fences replacement against edits made after async search started', () => { results.mockReturnValue({ matches: [ 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 fb06627ac55..dc7e5a623d5 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 @@ -144,6 +144,7 @@ export function usePierreDiffFind({ return } const nextSide = replace && isEditable ? 'additions' : sideRef.current + sideRef.current = nextSide setSide(nextSide) if (replace && isEditable && nextSide === 'additions') { setReplaceOpen(true) diff --git a/src/renderer/src/components/editor/pierre-diff/use-pierre-file-diff.test.tsx b/src/renderer/src/components/editor/pierre-diff/use-pierre-file-diff.test.tsx index c5f6e132d53..219857c7b27 100644 --- a/src/renderer/src/components/editor/pierre-diff/use-pierre-file-diff.test.tsx +++ b/src/renderer/src/components/editor/pierre-diff/use-pierre-file-diff.test.tsx @@ -130,6 +130,19 @@ it('surfaces a detached highlight failure so the retry affordance still appears' expect(result.current.fileDiff).toBe(diff) }) +it('surfaces a highlight failure that settles before the parse snapshot commits', async () => { + // Why: !pool.isWorkingPool() rejects synchronously, so highlight.catch runs before + // requestPierreFileDiff's caller commits { error: null }. + vi.mocked(requestPierreFileDiff).mockImplementation(async (_input, _signal, _block, onError) => { + onError?.(new Error('worker died')) + return diff + }) + const { result } = renderHook(() => usePierreFileDiff(input)) + await act(async () => vi.runOnlyPendingTimers()) + expect(result.current.error).toBe('worker died') + expect(result.current.fileDiff).toBe(diff) +}) + it('does not re-parse when only editability flips, but primes the highlight', async () => { vi.mocked(requestPierreFileDiff).mockResolvedValue(diff) let finishPrime: (value: void) => void = () => {} diff --git a/src/renderer/src/components/editor/pierre-diff/use-pierre-file-diff.ts b/src/renderer/src/components/editor/pierre-diff/use-pierre-file-diff.ts index f372dc0ffd5..6f77edd7e07 100644 --- a/src/renderer/src/components/editor/pierre-diff/use-pierre-file-diff.ts +++ b/src/renderer/src/components/editor/pierre-diff/use-pierre-file-diff.ts @@ -37,21 +37,26 @@ export function usePierreFileDiff(input: PierreDiffInput | null, editable = fals // Edits already paint through Pierre; coalesce parent echoes before recomputing. const timer = setTimeout( () => { + // Why: an already-rejected highlight enqueues onHighlightError before this then + // commits the snapshot, so the error must be captured for both orderings. + let highlightError: string | null = null void requestPierreFileDiff( input, controller.signal, editableRef.current, - (error: unknown) => + (error: unknown) => { + highlightError = error instanceof Error ? error.message : String(error) setSnapshot((previous) => previous && previous.input === input - ? { ...previous, error: error instanceof Error ? error.message : String(error) } + ? { ...previous, error: highlightError } : previous ) + } ).then( (diff) => { if (!controller.signal.aborted) { renderedScopeRef.current = JSON.stringify([input.cacheKey, input.path]) - setSnapshot({ input, diff, error: null }) + setSnapshot({ input, diff, error: highlightError }) } }, (error: unknown) => {