From cfe243af69ea9181ea98f612cd3881844891c207 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:35:35 -0700 Subject: [PATCH] fix(diff): restore read-only and original-side search with cancellable matching --- config/patches/@pierre__diffs@1.4.1.patch | 18 + config/patches/pierre-diffs.md | 4 + pnpm-lock.yaml | 6 +- .../editor/markdown-preview-search.ts | 2 +- .../pierre-diff/PierreDiffSearchBar.tsx | 199 +++++++++++ .../editor/pierre-diff/PierreDiffSurface.tsx | 28 +- .../editor/pierre-diff/pierre-diff-options.ts | 3 +- .../pierre-diff/pierre-diff-search-view.ts | 154 +++++++++ .../pierre-diff/pierre-diff-search.test.ts | 45 +++ .../editor/pierre-diff/pierre-diff-search.ts | 87 +++++ .../pierre-diff/pierre-diff-search.worker.ts | 17 + .../pierre-diff/use-pierre-diff-find.test.tsx | 130 +++++--- .../pierre-diff/use-pierre-diff-find.ts | 311 +++++++++++------- .../use-pierre-diff-search-results.test.ts | 62 ++++ .../use-pierre-diff-search-results.ts | 52 +++ .../use-pierre-diff-search-view.ts | 96 ++++++ tests/e2e/diff-search-parity.spec.ts | 152 +++++++++ 17 files changed, 1181 insertions(+), 185 deletions(-) create mode 100644 src/renderer/src/components/editor/pierre-diff/PierreDiffSearchBar.tsx create mode 100644 src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.ts create mode 100644 src/renderer/src/components/editor/pierre-diff/pierre-diff-search.test.ts create mode 100644 src/renderer/src/components/editor/pierre-diff/pierre-diff-search.ts create mode 100644 src/renderer/src/components/editor/pierre-diff/pierre-diff-search.worker.ts create mode 100644 src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-results.test.ts create mode 100644 src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-results.ts create mode 100644 src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-view.ts create mode 100644 tests/e2e/diff-search-parity.spec.ts diff --git a/config/patches/@pierre__diffs@1.4.1.patch b/config/patches/@pierre__diffs@1.4.1.patch index ed6a0f0e2a0..8dfaca27f8b 100644 --- a/config/patches/@pierre__diffs@1.4.1.patch +++ b/config/patches/@pierre__diffs@1.4.1.patch @@ -37,6 +37,24 @@ index 9bdaaa3ef199f3e443f77106a9ac7eddcc4b05b6..461364d3af60a954a766ef285ddef147 const fileDiff = this.getLatestDiff(); if (fileDiff == null || fileDiff.editSessionDirty !== true) return false; const { collapsedContextThreshold = 1 } = this.options; +diff --git a/dist/index.d.ts b/dist/index.d.ts +index 10626ccf2c08151951c7a42a7a014ba4b147869b..175f05ac3a4638d5f3df8f3f09b432da3d61e61c 100644 +--- a/dist/index.d.ts ++++ b/dist/index.d.ts +@@ -1,3 +1,4 @@ ++export { iterateOverDiff } from "./utils/iterateOverDiff.js"; + import { AnnotationLineMap, AnnotationSide, AnnotationSpan, AppliedThemeStyleCache, BaseCodeOptions, BaseDiffOptions, BaseDiffOptionsWithDefaults, BundledLanguage, ChangeContent, ChangeTypes, CodeColumnType, CodeToHastOptions, CodeViewDiffItem, CodeViewFileItem, CodeViewItem, CodeViewItemScrollTarget, CodeViewLayout, CodeViewLineScrollTarget, CodeViewPositionScrollTarget, CodeViewRangeScrollTarget, CodeViewScrollBehavior, CodeViewScrollTarget, ConflictResolverTypes, ContextContent, CreatePatchOptionsNonabortable, CustomPreProperties, DecorationItem, DiffAcceptRejectHunkConfig, DiffAcceptRejectHunkType, DiffFileInput, DiffIndicators, DiffLineAnnotation, DiffLineEventBaseProps, DiffTokenEventBaseProps, DiffsHighlighter, DiffsThemeNames, ExpansionDirections, ExtensionFormatMap, FileContents, FileDiffContentsLoader, FileDiffLoadedChangedFiles, FileDiffLoadedFiles, FileDiffLoadedPureRenamedFile, FileDiffMetadata, FileHeaderRenderMode, ForceDiffPlainTextOptions, ForceFilePlainTextOptions, GapSpan, HighlightedToken, HighlighterTypes, Hunk, HunkData, HunkExpansionRegion, HunkLineType, HunkSeparators, LanguageRegistration, LineAnnotation, LineDiffTypes, LineEventBaseProps, LineInfo, LineSpans, LineTypes, MaybeDiffFileInput, MergeConflictActionPayload, MergeConflictMarkerRow, MergeConflictMarkerRowType, MergeConflictRegion, MergeConflictResolution, NumericScrollLineAnchor, ObservedAnnotationNodes, ObservedGridNodes, ParsedPatch, PendingCodeViewLayoutReset, PostRenderPhase, PrePropertiesConfig, ProcessFileConflictData, RenderDiffFilesResult, RenderDiffOptions, RenderDiffResult, RenderFileMetadata, RenderFileOptions, RenderFileResult, RenderHeaderFilenameSuffixCallback, RenderHeaderMetadataCallback, RenderHeaderPrefixCallback, RenderRange, RenderWindow, RenderedDiffASTCache, RenderedFileASTCache, SelectedLineRange, SelectionPoint, SelectionSide, SharedRenderState, ShikiTransformer, SmoothScrollSettings, StickySpecs, SupportedLanguages, ThemeRegistration, ThemeRegistrationResolved, ThemeTypes, ThemedDiffResult, ThemedFileResult, ThemedToken, ThemesType, TokenEventBase, VirtualFileMetrics, VirtualWindowSpecs } from "./types.js"; + import { FileDiffEditCompleteEvent, FileEditCompleteEvent } from "./editor/types.js"; + import { GetHoveredLineResult, GetLineIndexUtility, InteractionManager, InteractionManagerBaseOptions, InteractionManagerMode, InteractionManagerOptions, LogTypes, MergeConflictActionTarget, OnDiffLineClickProps, OnDiffLineEnterLeaveProps, OnLineClickProps, OnLineEnterLeaveProps, OnTokenEventProps, SelectionWriteOptions, pluckInteractionOptions } from "./managers/InteractionManager.js"; +diff --git a/dist/index.js b/dist/index.js +index 5fa10ffa775e8f5e933b6fa415935ef6ab11c4dd..d64b134796b5dcf9bfb167c5ec700d9ea6a90c08 100644 +--- a/dist/index.js ++++ b/dist/index.js +@@ -1,3 +1,4 @@ ++export { iterateOverDiff } from "./utils/iterateOverDiff.js"; + import { ALTERNATE_FILE_NAMES_GIT, CODE_VIEW_FOOTER_ATTRIBUTE, CODE_VIEW_HEADER_ATTRIBUTE, COMMIT_METADATA_SPLIT, CORE_CSS_ATTRIBUTE, CUSTOM_HEADER_SLOT_ID, DEFAULT_CODE_VIEW_FILE_METRICS, DEFAULT_CODE_VIEW_LAYOUT, DEFAULT_COLLAPSED_CONTEXT_THRESHOLD, DEFAULT_EXPANDED_REGION, DEFAULT_RENDER_RANGE, DEFAULT_SMOOTH_SCROLL_SETTINGS, DEFAULT_THEMES, DEFAULT_TOKENIZE_MAX_LENGTH, DEFAULT_VIRTUAL_FILE_METRICS, DIFFS_DEVELOPMENT_BUILD, DIFFS_SCROLLBAR_GUTTER_MEASURED_PROPERTY, DIFFS_SCROLLBAR_MEASURE_ATTRIBUTE, DIFFS_TAG_NAME, EMPTY_RENDER_RANGE, FILENAME_HEADER_REGEX, FILENAME_HEADER_REGEX_GIT, FILE_CONTEXT_BLOB, GIT_DIFF_FILE_BREAK_REGEX, HEADER_FILENAME_SUFFIX_SLOT_ID, HEADER_METADATA_SLOT_ID, HEADER_PREFIX_SLOT_ID, HUNK_HEADER, INDEX_LINE_METADATA, MERGE_CONFLICT_BASE_MARKER_REGEX, MERGE_CONFLICT_END_MARKER_REGEX, MERGE_CONFLICT_SEPARATOR_MARKER_REGEX, MERGE_CONFLICT_START_MARKER_REGEX, SPLIT_WITH_NEWLINES, THEME_CSS_ATTRIBUTE, UNIFIED_DIFF_FILE_BREAK_REGEX, UNSAFE_CSS_ATTRIBUTE } from "./constants.js"; + import { AttachedLanguages, RegisteredCustomLanguages, ResolvedLanguages, ResolvingLanguages } from "./highlighter/languages/constants.js"; + import { areLanguagesAttached } from "./highlighter/languages/areLanguagesAttached.js"; diff --git a/dist/managers/InteractionManager.d.ts b/dist/managers/InteractionManager.d.ts index 9f243f97a6eb28c22176b6b3abba022756571fdc..806bb52ac15de68044ea5a5cfbb5016eae38ccae 100644 --- a/dist/managers/InteractionManager.d.ts diff --git a/config/patches/pierre-diffs.md b/config/patches/pierre-diffs.md index 3599ab2ca5a..b8c743304f8 100644 --- a/config/patches/pierre-diffs.md +++ b/config/patches/pierre-diffs.md @@ -24,3 +24,7 @@ Drag completion rechecks the predicate. Other consumers retain the upstream defa Keep `useTokenTransformer: true` on the pool. Coverage lives in `pierre-diff-worker-edit-cache.test.ts` and the large-diff Electron specs. Remove the patch when an upstream release provides the same behavior. + +The package entrypoint also exposes its existing `iterateOverDiff` iterator. +Search uses it to map original/context line numbers onto virtualized split and +unified rows, reusing Pierre's hunk logic without copying its implementation. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cc14a46a4f1..1eb8a870c52 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -109,7 +109,7 @@ overrides: monaco-editor>dompurify: 3.4.13 patchedDependencies: - '@pierre/diffs@1.4.1': cbcdcdfecdf4f666a96870b0bac2297cdb8cbcc5fbb12aece2280772aa8f8362 + '@pierre/diffs@1.4.1': cf3934292c1f82103136fce3b19bfe68097c39175218fa78c2ac3bbc3f332b0d '@vscode/windows-process-tree@0.8.0': f8ea245391c94da5770045aeea01fa6de466c2199c6ef46b5b769b398aa9823e '@xterm/addon-ligatures@0.11.0-beta.300': 47405b9994b5acf1b4e90b49250358c1ca03649854d59560e7732b72fe336920 '@xterm/addon-search@0.17.0-beta.300': eee5338dd2621ece46e79c61ec06766cd7fadaf79ffdb24e2a8ab68e97ef31f0 @@ -143,7 +143,7 @@ importers: version: 2.5.6 '@pierre/diffs': specifier: 1.4.1 - version: 1.4.1(patch_hash=cbcdcdfecdf4f666a96870b0bac2297cdb8cbcc5fbb12aece2280772aa8f8362)(@shikijs/themes@4.4.3)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + version: 1.4.1(patch_hash=cf3934292c1f82103136fce3b19bfe68097c39175218fa78c2ac3bbc3f332b0d)(@shikijs/themes@4.4.3)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) '@xterm/addon-serialize': specifier: 0.15.0-beta.300 version: 0.15.0-beta.300(patch_hash=851eac3d75e6d8c013b9f4c053e61d824b23965cb19ecc28e335e05059f3a294)(@xterm/xterm@6.1.0-beta.303(patch_hash=98756bcedc402bcdb7c6ab7b015d2e59cd18e97b03a2c06a27e95bb3ba429d9d)) @@ -8280,7 +8280,7 @@ snapshots: tslib: 2.8.1 webcrypto-core: 1.9.2 - '@pierre/diffs@1.4.1(patch_hash=cbcdcdfecdf4f666a96870b0bac2297cdb8cbcc5fbb12aece2280772aa8f8362)(@shikijs/themes@4.4.3)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)': + '@pierre/diffs@1.4.1(patch_hash=cf3934292c1f82103136fce3b19bfe68097c39175218fa78c2ac3bbc3f332b0d)(@shikijs/themes@4.4.3)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)': dependencies: '@pierre/theme': 2.0.0 '@pierre/theming': 1.0.1(@pierre/theme@2.0.0)(@shikijs/themes@4.4.3)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)(shiki@4.4.3) diff --git a/src/renderer/src/components/editor/markdown-preview-search.ts b/src/renderer/src/components/editor/markdown-preview-search.ts index 1b92f958fa8..2763873275d 100644 --- a/src/renderer/src/components/editor/markdown-preview-search.ts +++ b/src/renderer/src/components/editor/markdown-preview-search.ts @@ -133,7 +133,7 @@ function codePointAt(text: string, index: number): string | undefined { return codePoint === undefined ? undefined : String.fromCodePoint(codePoint) } -function isWholeWordMatch(text: string, start: number, end: number): boolean { +export function isWholeWordMatch(text: string, start: number, end: number): boolean { const before = codePointBefore(text, start) const after = codePointAt(text, end) return !isWordCharacter(before) && !isWordCharacter(after) diff --git a/src/renderer/src/components/editor/pierre-diff/PierreDiffSearchBar.tsx b/src/renderer/src/components/editor/pierre-diff/PierreDiffSearchBar.tsx new file mode 100644 index 00000000000..d4a491d2592 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/PierreDiffSearchBar.tsx @@ -0,0 +1,199 @@ +import { + CaseSensitive, + ChevronDown, + ChevronRight, + ChevronUp, + Regex, + Replace, + ReplaceAll, + WholeWord, + X +} from 'lucide-react' +import { Button } from '@/components/ui/button' +import { Input } from '@/components/ui/input' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' +import type { DiffSearchQuery } from './pierre-diff-search' + +export type DiffSearchSide = 'additions' | 'deletions' +export type PierreDiffSearchBarProps = { + inputRef: React.RefObject + query: DiffSearchQuery + side: DiffSearchSide + replacement: string + replaceOpen: boolean + canReplace: boolean + canNavigate: boolean + canReplaceAll: boolean + status: string + onQuery: (query: DiffSearchQuery) => void + onSide: (side: DiffSearchSide) => void + onReplacement: (text: string) => void + onToggleReplace: () => void + onReplace: (all: boolean) => void + onNavigate: (direction: 1 | -1) => void + onClose: () => void +} + +function SearchButton({ + label, + children, + ...props +}: React.ComponentProps & { label: string }) { + return ( + + + + + {label} + + ) +} + +export function PierreDiffSearchBar(props: PierreDiffSearchBarProps) { + const { query, onQuery } = props + return ( +
+
event.stopPropagation()} + > +
+ {props.canReplace && ( + + {props.replaceOpen ? : } + + )} + onQuery({ ...query, text: event.target.value })} + onKeyDown={(event) => { + if (isImeCompositionKeyDown(event)) { + return + } + if (event.key === 'Enter') { + event.preventDefault() + props.onNavigate(event.shiftKey ? -1 : 1) + } + if (event.key === 'Escape') { + event.preventDefault() + props.onClose() + } + }} + /> + {( + [ + ['matchCase', 'Match case', CaseSensitive], + ['wholeWord', 'Match whole word', WholeWord], + ['regex', 'Use regular expression', Regex] + ] as const + ).map(([key, label, Icon]) => ( + onQuery({ ...query, [key]: !query[key] })} + > + + + ))} + props.onNavigate(-1)} + > + + + props.onNavigate(1)} + > + + + + + +
+
+ {( + [ + ['deletions', 'Original'], + ['additions', 'Modified'] + ] as const + ).map(([side, label]) => ( + + ))} + + {props.status} + +
+ {props.canReplace && props.replaceOpen && ( +
+ props.onReplacement(event.target.value)} + onKeyDown={(event) => { + if (isImeCompositionKeyDown(event)) { + return + } + if (event.key === 'Enter') { + event.preventDefault() + props.onReplace(false) + } + if (event.key === 'Escape') { + event.preventDefault() + props.onClose() + } + }} + /> + props.onReplace(false)} + > + + + props.onReplace(true)} + > + + +
+ )} +
+
+ ) +} diff --git a/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx index e2e7f596973..6623e64fe86 100644 --- a/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx +++ b/src/renderer/src/components/editor/pierre-diff/PierreDiffSurface.tsx @@ -25,6 +25,7 @@ import { type PierreDiffCommentAnnotation } from './pierre-diff-comment-annotations' import { usePierreDiffFind } from './use-pierre-diff-find' +import { PierreDiffSearchBar } from './PierreDiffSearchBar' import { installPierreContextualCopy } from './pierre-diff-context-copy' import { editorShortcutMatches } from '../editor-shortcuts' import { usePierreDiffNoteNavigation } from './use-pierre-diff-note-navigation' @@ -103,9 +104,17 @@ export function PierreDiffSurface({ const containerRef = useRef(null) const editorRef = useRef | null>(null) const onEditChangeRef = useRef(onEditChange) - onEditChangeRef.current = onEditChange - const { editEnabled, handleContainerKeyDown, handleContainerBlur, handleEditorAttach } = - usePierreDiffFind({ isEditable, containerRef }) + const { + searchBar, + handleContainerKeyDown, + onPointerDown, + onPostRender: searchPostRender, + onEditChange: searchEditChange + } = usePierreDiffFind({ isEditable, containerRef, editorRef, fileDiff }) + onEditChangeRef.current = (file) => { + onEditChange?.(file) + searchEditChange() + } const navigateToNote = usePierreDiffNoteNavigation({ worktreeId, filePath, comments }) const commentableLines = useMemo( () => (commentableLineNumbers ? new Set(commentableLineNumbers) : null), @@ -148,6 +157,7 @@ export function PierreDiffSurface({ onPostRender: (node: HTMLElement, instance: PierreDiffInstance, phase: PostRenderPhase) => { onPostRender?.(node, phase, instance) navigateToNote(node, phase, instance) + searchPostRender(node, phase, instance) } }), [ @@ -157,6 +167,7 @@ export function PierreDiffSurface({ onPostRender, onAddComment, navigateToNote, + searchPostRender, commentableLines, addCommentLabel ] @@ -179,14 +190,10 @@ export function PierreDiffSurface({ { onAttach: (editor) => { editorRef.current = editor - handleEditorAttach(editor) }, onComplete: () => { editorRef.current = null }, - // Why: Cmd+F opens edit mode even on read-only diffs, so ignore changes - // unless this surface can actually save. Otherwise a stray keystroke in - // the find panel marks a staged or branch section dirty with no save path. onChange: (event) => { if (isEditable) { onEditChangeRef.current?.(event.file) @@ -196,7 +203,7 @@ export function PierreDiffSurface({ isEditable ? editStateKey : undefined, fileDiff ), - [handleEditorAttach, isEditable, editStateKey, fileDiff] + [isEditable, editStateKey, fileDiff] ) const renderAnnotation = useCallback( (annotation: PierreDiffCommentAnnotation) => @@ -242,6 +249,7 @@ export function PierreDiffSurface({ event.currentTarget.focus({ preventScroll: true }) } }} + onPointerDownCapture={onPointerDown} onPointerUp={() => { // Pierre must consume the native caret before a keystroke can arrive. if (isEditable) { @@ -258,8 +266,8 @@ export function PierreDiffSurface({ } handleContainerKeyDown(event) }} - onBlur={handleContainerBlur} > + {searchBar && } {/* Why: @pierre/diffs throws from its own ref teardown on some remounts ("A FileDiff instance should exist when unmounting"). Contain it to the one file instead of letting an experimental dependency take down the @@ -277,7 +285,7 @@ export function PierreDiffSurface({ options={options} style={style} metrics={metrics} - edit={editEnabled} + edit={isEditable} editorOptions={editorOptions} lineAnnotations={lineAnnotations} renderAnnotation={renderAnnotation} diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-options.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-options.ts index 26e182f5cb6..d395397c4ea 100644 --- a/src/renderer/src/components/editor/pierre-diff/pierre-diff-options.ts +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-options.ts @@ -3,6 +3,7 @@ import { DEFAULT_VIRTUAL_FILE_METRICS, type FileDiffOptions, type ThemesType } f import type { CreatePatchOptionsNonabortable } from 'diff' import type { GlobalSettings } from '../../../../../shared/global-settings-types' import { computeDiffEditorFontSize, resolveEditorFontFamily } from '@/lib/editor-font-zoom' +import { PIERRE_SEARCH_CSS } from './pierre-diff-search-view' import { buildFontFamily } from '@/components/terminal-pane/layout-serialization' /** @@ -62,7 +63,7 @@ export function buildPierreDiffOptions({ // from here — without it Pierre asks Shiki for its unregistered default // `pierre-dark` and the render throws instead of painting. theme: PIERRE_DIFF_THEMES, - unsafeCSS: PIERRE_GUTTER_BUTTON_CSS, + unsafeCSS: PIERRE_GUTTER_BUTTON_CSS + PIERRE_SEARCH_CSS, themeType: settings?.theme ?? 'system', overflow: settings?.diffWordWrap ? 'wrap' : 'scroll', parseDiffOptions: buildPierreParseDiffOptions(settings?.diffShowWhitespace), 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 new file mode 100644 index 00000000000..7162d3dc7aa --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search-view.ts @@ -0,0 +1,154 @@ +import { iterateOverDiff, type FileDiffMetadata } from '@pierre/diffs' +import type { DiffSearchMatch } from './pierre-diff-search' +import type { DiffSearchSide } from './PierreDiffSearchBar' + +export const PIERRE_SEARCH_CSS = ` +::highlight(orca-diff-find) { background-color: color-mix(in srgb, var(--markdown-search-match) 50%, transparent); color: inherit; } +::highlight(orca-diff-find-active) { background-color: color-mix(in srgb, var(--markdown-search-match-active) 60%, transparent); color: inherit; } +` +const highlights = new Map() + +export function paintPierreSearchHighlights( + owner: object, + ranges?: { matches: Range[]; active: Range[] } +) { + if (ranges) { + highlights.set(owner, ranges) + } else { + highlights.delete(owner) + } + if (typeof Highlight === 'undefined' || !CSS.highlights) { + return + } + for (const [name, key] of [ + ['orca-diff-find', 'matches'], + ['orca-diff-find-active', 'active'] + ] as const) { + const highlight = new Highlight() + for (const entry of highlights.values()) { + for (const range of entry[key]) { + highlight.add(range) + } + } + if (highlight.size) { + CSS.highlights.set(name, highlight) + } else { + CSS.highlights.delete(name) + } + } +} + +export function pierreRowTextRange(row: HTMLElement, start: number, end: number): Range | null { + const walker = document.createTreeWalker(row, NodeFilter.SHOW_TEXT) + const range = document.createRange() + let offset = 0 + let started = false + let node: Node | null + while ((node = walker.nextNode())) { + const length = node.textContent?.length ?? 0 + if (!started && offset + length >= start) { + range.setStart(node, Math.max(0, start - offset)) + started = true + } + if (started && offset + length >= end) { + range.setEnd(node, Math.max(0, end - offset)) + return range + } + offset += length + } + return null +} + +export function getPierreSearchRanges( + host: HTMLElement, + diff: FileDiffMetadata, + side: DiffSearchSide, + matches: DiffSearchMatch[], + active?: DiffSearchMatch +) { + const rows = [ + ...(host.shadowRoot?.querySelectorAll('[data-code] [data-line]') ?? []) + ] + const split = Boolean(host.shadowRoot?.querySelector('[data-code][data-deletions]')) + const indexPart = split ? 1 : 0 + const indexes = rows + .map((row) => Number(row.dataset.lineIndex?.split(',')[indexPart])) + .filter(Number.isFinite) + const lineByIndex = new Map() + if (indexes.length) { + iterateOverDiff({ + diff, + diffStyle: split ? 'split' : 'unified', + expandedHunks: true, + startingLine: Math.min(...indexes), + totalLines: Math.max(...indexes) - Math.min(...indexes) + 1, + callback: ({ additionLine, deletionLine }) => { + const line = side === 'additions' ? additionLine : deletionLine + if (line) { + lineByIndex.set(split ? line.splitLineIndex : line.unifiedLineIndex, line.lineNumber) + } + } + }) + } + const result: { matches: Range[]; active: Range[] } = { matches: [], active: [] } + for (const row of rows) { + if (split && !row.closest(`[data-code][data-${side}]`)) { + continue + } + const line = lineByIndex.get(Number(row.dataset.lineIndex?.split(',')[indexPart])) + if (line === undefined) { + continue + } + // Matches are ordered, so only visit those intersecting this mounted row. + let lo = 0, + hi = matches.length + while (lo < hi) { + const mid = (lo + hi) >>> 1 + if (matches[mid].range.end.line < line - 1) { + lo = mid + 1 + } else { + hi = mid + } + } + for (let i = lo; i < matches.length && matches[i].range.start.line <= line - 1; i++) { + const match = matches[i] + const start = match.range.start.line === line - 1 ? match.range.start.character : 0 + const end = + match.range.end.line === line - 1 + ? match.range.end.character + : (row.textContent?.length ?? 0) + const range = pierreRowTextRange(row, start, end) + if (range) { + result.matches.push(range) + if (match === active) { + result.active.push(range) + } + } + } + } + return result +} + +export function pierreSearchRevealLine( + diff: FileDiffMetadata, + lineNumber: number, + side: DiffSearchSide +): number { + if (side === 'additions') { + return lineNumber + } + let modifiedLine = lineNumber + iterateOverDiff({ + diff, + diffStyle: 'unified', + expandedHunks: true, + callback: ({ deletionLine, additionLine }) => { + if (deletionLine?.lineNumber !== lineNumber) { + return + } + modifiedLine = additionLine?.lineNumber ?? lineNumber + return true + } + }) + return modifiedLine +} diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.test.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.test.ts new file mode 100644 index 00000000000..3dbf74fe486 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.test.ts @@ -0,0 +1,45 @@ +import { expect, it } from 'vitest' +import { searchPierreDiff } from './pierre-diff-search' + +function search(text: string, query: string, options = {}, replacement = '') { + return searchPierreDiff({ + text, + query: { text: query, regex: false, matchCase: false, wholeWord: false, ...options }, + replacement + }) +} + +it('searches multiline text and maps CRLF positions', () => { + const result = search('before\r\nfirst\r\nsecond\r\nafter', 'first\r\nsecond') + expect(result.matches.map((match) => match.range)).toEqual([ + { start: { line: 1, character: 0 }, end: { line: 2, character: 6 } } + ]) +}) + +it('supports case, Unicode whole words and literal regex characters', () => { + expect(search('Cat cat catapult caté', 'cat', { wholeWord: true }).matches).toHaveLength(2) + expect(search('Cat cat', 'cat', { matchCase: true }).matches).toHaveLength(1) + expect(search('a.b axb', 'a.b').matches).toHaveLength(1) +}) + +it('expands capture replacements with lookbehind and named groups', () => { + const text = 'prefix key:42 suffix' + const pattern = '(?<=key:)(?\\d+)' + const template = "$-$1-$$-$&-$`-$'" + const match = search(text, pattern, { regex: true }, template).matches[0] + const replaced = text.slice(0, match.start) + match.replacement + text.slice(match.end) + expect(replaced).toBe(text.replace(new RegExp(pattern, 'gmu'), template)) +}) + +it('reports invalid expressions and makes progress through zero-width Unicode matches', () => { + expect(search('text', '[', { regex: true }).error).toBe('Invalid regular expression') + expect(search('😀a', '(?=.)', { regex: true }).matches.map((match) => match.start)).toEqual([ + 0, 2 + ]) +}) + +it('caps results without allowing replace-all to silently replace a prefix', () => { + const result = search('x '.repeat(10_002), 'x') + expect(result.matches).toHaveLength(10_000) + expect(result.truncated).toBe(true) +}) diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.ts new file mode 100644 index 00000000000..f25261a58c2 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.ts @@ -0,0 +1,87 @@ +import { TextDocument, type Range } from '@pierre/diffs/edit' +import { isWholeWordMatch } from '../markdown-preview-search' + +export type DiffSearchQuery = { + text: string + matchCase: boolean + wholeWord: boolean + regex: boolean +} +export type DiffSearchMatch = { range: Range; start: number; end: number; replacement: string } +export type DiffSearchRequest = { text: string; query: DiffSearchQuery; replacement: string } +export type DiffSearchResult = { matches: DiffSearchMatch[]; truncated: boolean; error?: string } +export const MAX_DIFF_SEARCH_MATCHES = 10_000 + +function replacementText(template: string, match: RegExpExecArray, text: string): string { + return template.replace(/\$(\$|&|`|'|\d{1,2}|<[^>]+>)/g, (token, name: string) => { + if (name === '$') { + return '$' + } + if (name === '&') { + return match[0] + } + if (name === '`') { + return text.slice(0, match.index) + } + if (name === "'") { + return text.slice(match.index + match[0].length) + } + if (name.startsWith('<')) { + return match.groups ? (match.groups[name.slice(1, -1)] ?? '') : token + } + const index = Number(name) + if (index > 0 && index < match.length) { + return match[index] ?? '' + } + const first = Number(name[0]) + return name.length === 2 && first > 0 && first < match.length + ? (match[first] ?? '') + name[1] + : token + }) +} + +// Runs only in a terminable worker; even pathological regexes cannot block typing. +export function searchPierreDiff({ + text, + query, + replacement +}: DiffSearchRequest): DiffSearchResult { + if (!query.text) { + return { matches: [], truncated: false } + } + const source = query.regex ? query.text : query.text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + let pattern: RegExp + try { + pattern = new RegExp(source, query.matchCase ? 'gmu' : 'gimu') + } catch { + return { matches: [], truncated: false, error: 'Invalid regular expression' } + } + const document = new TextDocument('diff-search', text) + const matches: DiffSearchMatch[] = [] + let replacementCharacters = 0 + let match: RegExpExecArray | null + while ((match = pattern.exec(text))) { + const start = match.index + const end = start + match[0].length + if (!query.wholeWord || isWholeWordMatch(text, start, end)) { + if (matches.length === MAX_DIFF_SEARCH_MATCHES) { + return { matches, truncated: true } + } + const nextReplacement = query.regex ? replacementText(replacement, match, text) : replacement + replacementCharacters += nextReplacement.length + if (replacementCharacters > 16_000_000) { + return { matches: [], truncated: false, error: 'Replacement is too large' } + } + matches.push({ + start, + end, + range: { start: document.positionAt(start), end: document.positionAt(end) }, + replacement: nextReplacement + }) + } + if (start === end) { + pattern.lastIndex += (text.codePointAt(end) ?? 0) > 0xffff ? 2 : 1 + } + } + return { matches, truncated: false } +} diff --git a/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.worker.ts b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.worker.ts new file mode 100644 index 00000000000..2e9ebf9f21e --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/pierre-diff-search.worker.ts @@ -0,0 +1,17 @@ +import { + searchPierreDiff, + type DiffSearchRequest, + type DiffSearchResult +} from './pierre-diff-search' + +const scope = globalThis as unknown as { + onmessage: (event: MessageEvent) => void + postMessage: (response: DiffSearchResult) => void +} +scope.onmessage = ({ data }) => { + try { + scope.postMessage(searchPierreDiff(data)) + } catch { + scope.postMessage({ matches: [], truncated: false, error: 'Could not search this file' }) + } +} 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 3739c3e0cf7..f859817e385 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 @@ -1,72 +1,118 @@ // @vitest-environment happy-dom import { act, renderHook, cleanup } from '@testing-library/react' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, expect, it, vi } from 'vitest' +import type { FileDiffMetadata } from '@pierre/diffs' import { usePierreDiffFind } from './use-pierre-diff-find' +const { results } = vi.hoisted(() => ({ results: vi.fn() })) +vi.mock('./use-pierre-diff-search-results', () => ({ usePierreDiffSearchResults: results })) +vi.mock('./use-pierre-diff-search-view', () => ({ usePierreDiffSearchView: () => vi.fn() })) vi.mock('../editor-shortcuts', () => ({ - editorShortcutMatches: (_: string, event: KeyboardEvent) => event.key === 'f' && event.ctrlKey + editorShortcutMatches: (action: string, event: KeyboardEvent) => + event.key === (action === 'editor.find' ? 'f' : 'h') && event.ctrlKey })) -vi.mock('@/lib/shortcut-platform', () => ({ getShortcutPlatform: () => 'linux' })) afterEach(() => { cleanup() - vi.useRealTimers() + vi.clearAllMocks() 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 editor = { + getText: vi.fn(() => 'modified'), + applyEdits: vi.fn(), + setSelections: vi.fn(), + focus: vi.fn() } - const find = () => + const fileDiff = { additionLines: ['modified'], deletionLines: ['original'] } as FileDiffMetadata + const containerRef = { current: container } + const editorRef = { current: editor } as never + const { result } = renderHook(() => + usePierreDiffFind({ + isEditable, + containerRef, + editorRef, + fileDiff + }) + ) + const find = (key = 'f') => act(() => result.current.handleContainerKeyDown({ - key: 'f', + key, + nativeEvent: new KeyboardEvent('keydown', { key, ctrlKey: true }), ctrlKey: true, preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.KeyboardEvent) ) - return { result, attachContent, find } + return { result, editor, 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 find on the first press without creating a writable read-only session', () => { + results.mockReturnValue(null) + const { result, find, editor } = setup(false) + find() + expect(result.current.searchBar?.canReplace).toBe(false) + act(() => result.current.searchBar?.onReplace(true)) + expect(editor.applyEdits).not.toHaveBeenCalled() + act(() => result.current.searchBar?.onClose()) + expect(result.current.searchBar).toBeNull() +}) + +it('searches original content and never replaces on that side', () => { + results.mockReturnValue(null) + const { result, find } = setup(true) + find() + act(() => result.current.searchBar?.onSide('deletions')) + expect(results.mock.lastCall?.[0].text).toBe('original') + expect(result.current.searchBar?.canReplace).toBe(false) +}) + +it('fences replacement against edits made after async search started', () => { + results.mockReturnValue({ + matches: [ + { + range: { start: { line: 1, character: 0 }, end: { line: 1, character: 8 } }, + replacement: 'new' + } + ], + truncated: false }) + const { result, find, editor } = setup(true) + find('h') + expect(result.current.searchBar?.replaceOpen).toBe(true) + editor.getText.mockReturnValue('newer edit') + act(() => result.current.searchBar?.onReplace(true)) + expect(editor.applyEdits).not.toHaveBeenCalled() +}) - 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() })) +it('moves between results with F3 and Shift+F3', () => { + const range = { start: { line: 0, character: 0 }, end: { line: 0, character: 1 } } + results.mockReturnValue({ matches: [{ range }, { range }], truncated: false }) + const { result, find } = setup(false) + find() + act(() => + result.current.searchBar?.onQuery({ + text: 'm', + regex: false, + matchCase: false, + wholeWord: false + }) + ) + const key = (shiftKey: boolean) => act(() => - result.current.handleContainerKeyDown({ key: 'Escape' } as React.KeyboardEvent) + result.current.handleContainerKeyDown({ + key: 'F3', + shiftKey, + preventDefault: vi.fn(), + stopPropagation: vi.fn() + } as unknown as React.KeyboardEvent) ) - act(() => vi.runOnlyPendingTimers()) - expect(search).not.toHaveBeenCalled() - expect(result.current.editEnabled).toBe(false) - }) + key(false) + expect(result.current.searchBar?.status).toBe('2/2') + key(true) + expect(result.current.searchBar?.status).toBe('1/2') }) 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 443e4d13070..199f8f90e57 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,162 +1,217 @@ -import { useCallback, useEffect, useRef, useState } from 'react' -import type { EditorFocusOptions } from '@pierre/diffs/edit' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { resolveFindAgainShortcut, type Editor } from '@pierre/diffs/edit' +import type { FileDiffMetadata } from '@pierre/diffs' import { getShortcutPlatform } from '@/lib/shortcut-platform' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { editorShortcutMatches } from '../editor-shortcuts' +import type { PierreDiffAnnotationData } from './pierre-diff-comment-annotations' +import type { DiffSearchQuery } from './pierre-diff-search' +import type { DiffSearchSide, PierreDiffSearchBarProps } from './PierreDiffSearchBar' +import { usePierreDiffSearchResults } from './use-pierre-diff-search-results' +import { usePierreDiffSearchView } from './use-pierre-diff-search-view' -/** - * Pierre only accepts key events whose target is its own content element, so - * reach into the shadow root rather than trusting whatever holds focus — at - * attach time focus has not landed there yet. - */ -function findPierreContentElement(container: HTMLElement | null): HTMLElement | null { - const host = container?.querySelector('diffs-container') - const editable = host?.shadowRoot?.querySelector('[contenteditable="true"]') - return editable instanceof HTMLElement ? editable : null -} +type DiffEditor = Editor<'file-diff', PierreDiffAnnotationData, undefined> -// Why: Pierre only ships its search panel with edit mode, and it has no -// programmatic command dispatch — replay the shortcut its own listener expects. -function dispatchPierreOpenSearchPanel(target: HTMLElement): void { - const isMac = getShortcutPlatform() === 'darwin' - target.dispatchEvent( - new KeyboardEvent('keydown', { - key: 'f', - code: 'KeyF', - metaKey: isMac, - ctrlKey: !isMac, - bubbles: true, - composed: true, - cancelable: true - }) - ) -} - -// Why: only `focus` is needed here, so stay structural and annotation-agnostic. -type FocusableEditor = { focus: (options?: EditorFocusOptions) => void } - -export type PierreDiffFind = { - /** True when the surface should mount an edit session (real editing or find). */ - editEnabled: boolean - /** Capture-phase keydown handler for the surface container. */ - handleContainerKeyDown: (event: React.KeyboardEvent) => void - /** Ends a find-only session when focus leaves the surface. */ - handleContainerBlur: (event: React.FocusEvent) => void - /** Pass to `editorOptions.onAttach` so the panel opens on the first press. */ - handleEditorAttach: (editor: FocusableEditor) => void - /** Leaves a find-only session so a read-only diff stops accepting input. */ - exitFind: () => void -} - -/** - * Bridges our ⌘F keybinding onto Pierre's edit-mode search panel. On a - * read-only diff the session exists only for find, and nothing is ever written - * back, so dismissing it discards any stray keystrokes. - */ export function usePierreDiffFind({ isEditable, - containerRef + containerRef, + editorRef, + fileDiff }: { isEditable: boolean containerRef: React.RefObject -}): 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) + editorRef: React.RefObject + fileDiff: FileDiffMetadata +}) { + const [open, setOpen] = useState(false) + const [side, setSide] = useState('additions') + const sideRef = useRef('additions') + const inputRef = useRef(null) + const [query, setQuery] = useState({ + text: '', + regex: false, + matchCase: false, + wholeWord: false + }) + const [replacement, setReplacement] = useState('') + const [replaceOpen, setReplaceOpen] = useState(false) + const [revision, setRevision] = useState(0) + const [selection, setSelection] = useState<{ request: object; index: number }>() + const request = useMemo( + () => + open + ? { + text: + side === 'deletions' + ? fileDiff.deletionLines.join('') + : (editorRef.current?.getText() ?? fileDiff.additionLines.join('')), + query, + replacement, + revision + } + : null, + [open, side, fileDiff, query, replacement, revision, editorRef] + ) + const result = usePierreDiffSearchResults(request) + const matches = result?.matches + const index = selection?.request === request ? selection.index : 0 + const active = matches?.[index] + const onPostRender = usePierreDiffSearchView({ fileDiff, side, matches, active }) + const canReplace = isEditable && side === 'additions' + useEffect(() => { + if (canReplace && active) { + editorRef.current?.setSelections([{ ...active.range, direction: 'forward' }]) } - findFrameRef.current = requestAnimationFrame(() => { - findFrameRef.current = null - const target = findPierreContentElement(containerRef.current) - if (!target || !pendingFindRef.current) { + }, [canReplace, active, editorRef]) + const close = useCallback(() => { + setOpen(false) + if (isEditable && side === 'additions') { + editorRef.current?.focus({ preventScroll: true }) + } else { + containerRef.current?.focus({ preventScroll: true }) + } + }, [isEditable, side, editorRef, containerRef]) + const navigate = useCallback( + (direction: 1 | -1) => { + if (!request || !matches?.length) { 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) - } + setSelection({ request, index: (index + direction + matches.length) % matches.length }) }, - [] + [request, matches, index] ) - + const replace = (all: boolean) => { + const editor = editorRef.current + // Async matching must never apply offsets from an older document or query. + if ( + !canReplace || + !editor || + !request || + !result || + result.error || + request.text !== editor.getText() || + (all && result.truncated) + ) { + return + } + const targets = all ? result.matches : active ? [active] : [] + if (!targets.length) { + return + } + editor.applyEdits(targets.map(({ range, replacement: newText }) => ({ range, newText }))) + setRevision((value) => value + 1) + } const handleContainerKeyDown = useCallback( (event: React.KeyboardEvent) => { - if (replayingFindRef.current) { + if (isImeCompositionKeyDown(event)) { 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') { - pendingFindRef.current = false - setFindActive(false) + if (open && event.key === 'Escape') { + event.preventDefault() + event.stopPropagation() + close() return } - if (!editorShortcutMatches('editor.find', event)) { + const again = + event.key === 'F3' + ? event.shiftKey + ? 'previous' + : 'next' + : resolveFindAgainShortcut(event.nativeEvent, getShortcutPlatform() === 'darwin') + if (open && again) { + event.preventDefault() + event.stopPropagation() + navigate(again === 'previous' ? -1 : 1) + return + } + const find = editorShortcutMatches('editor.find', event) + const replace = editorShortcutMatches('editor.replace', event) + if (!find && !replace) { 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, containerRef, openMountedSearch] - ) - - // Why: leaving the surface ends a find-only session too; the panel is gone. - const handleContainerBlur = useCallback( - (event: React.FocusEvent) => { - if (!findActive || event.currentTarget.contains(event.relatedTarget as Node | null)) { + if (event.repeat) { return } - pendingFindRef.current = false - setFindActive(false) - }, - [findActive] - ) - - const handleEditorAttach = useCallback( - (editor: FocusableEditor) => { - if (!pendingFindRef.current) { - return + const nextSide = replace && isEditable ? 'additions' : sideRef.current + setSide(nextSide) + if (replace && isEditable && nextSide === 'additions') { + setReplaceOpen(true) } - 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. - openMountedSearch() + setOpen(true) + const root = containerRef.current?.querySelector('diffs-container')?.shadowRoot + const selection = ( + root as ShadowRoot & { getSelection?: () => Selection | null } + )?.getSelection?.() + const selected = root?.contains(selection?.anchorNode ?? null) ? selection?.toString() : '' + if (selected) { + setQuery((value) => ({ ...value, text: selected })) + } + inputRef.current?.focus({ preventScroll: true }) + inputRef.current?.select() }, - [openMountedSearch] + [containerRef, isEditable, open, close, navigate] ) + useEffect(() => { + if (open) { + inputRef.current?.focus({ preventScroll: true }) + inputRef.current?.select() + } + }, [open]) - const exitFind = useCallback(() => { - pendingFindRef.current = false - setFindActive(false) - }, []) - + const searchBar: PierreDiffSearchBarProps | null = open + ? { + inputRef, + query, + side, + replacement, + replaceOpen, + canReplace, + canNavigate: Boolean(matches?.length), + canReplaceAll: Boolean(matches?.length) && !result?.truncated, + status: + result?.error ?? + (!query.text + ? '0/0' + : !result + ? 'Searching…' + : !matches?.length + ? 'No results' + : `${index + 1}/${matches.length}${result.truncated ? '+' : ''}`), + onQuery: setQuery, + onReplacement: setReplacement, + onToggleReplace: () => setReplaceOpen((value) => !value), + onSide: (value) => { + sideRef.current = value + setSide(value) + inputRef.current?.focus({ preventScroll: true }) + }, + onNavigate: navigate, + onReplace: replace, + onClose: close + } + : null return { - editEnabled: isEditable || findActive, + searchBar, handleContainerKeyDown, - handleContainerBlur, - handleEditorAttach, - exitFind + onPostRender, + onEditChange: () => { + if (open) { + setRevision((value) => value + 1) + } + }, + onPointerDown: (event: React.PointerEvent) => { + const path = event.nativeEvent.composedPath() + const original = path.some( + (node) => + node instanceof Element && + (node.matches('[data-code][data-deletions]') || + node.matches('[data-line-type="change-deletion"]')) + ) + if (path.some((node) => node instanceof Element && node.matches('[data-code]'))) { + sideRef.current = original ? 'deletions' : 'additions' + } + } } } diff --git a/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-results.test.ts b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-results.test.ts new file mode 100644 index 00000000000..b7414b0685c --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-results.test.ts @@ -0,0 +1,62 @@ +// @vitest-environment happy-dom +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import { usePierreDiffSearchResults } from './use-pierre-diff-search-results' + +const workers: FakeWorker[] = [] +class FakeWorker { + onmessage?: (event: { data: unknown }) => void + postMessage = vi.fn() + terminate = vi.fn() + constructor() { + workers.push(this) + } +} +function request(text: string) { + return { + text: 'contents', + query: { text, regex: true, matchCase: false, wholeWord: false }, + replacement: '' + } +} +function setup() { + vi.useFakeTimers() + vi.stubGlobal('Worker', FakeWorker) + const hook = renderHook(({ input }) => usePierreDiffSearchResults(input), { + initialProps: { input: request('first') } + }) + act(() => vi.advanceTimersByTime(120)) + return hook +} +afterEach(() => { + cleanup() + workers.length = 0 + vi.unstubAllGlobals() + vi.useRealTimers() +}) + +it('terminates abandoned regex work and rejects its late result', () => { + const hook = setup() + const first = workers[0] + hook.rerender({ input: request('second') }) + expect(first.terminate).toHaveBeenCalled() + act(() => first.onmessage?.({ data: { matches: ['stale'] } })) + expect(hook.result.current).toBeNull() + act(() => vi.advanceTimersByTime(120)) + act(() => workers[1].onmessage?.({ data: { matches: [], truncated: false } })) + expect(hook.result.current).toEqual({ matches: [], truncated: false }) + expect(workers[1].terminate).toHaveBeenCalled() +}) + +it('terminates a stalled worker and reports a recoverable timeout', () => { + const hook = setup() + act(() => vi.advanceTimersByTime(5_000)) + expect(workers[0].terminate).toHaveBeenCalled() + expect(hook.result.current?.error).toContain('Search took too long') +}) + +it('terminates active work on unmount', () => { + const hook = setup() + hook.unmount() + expect(workers[0].terminate).toHaveBeenCalled() +}) diff --git a/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-results.ts b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-results.ts new file mode 100644 index 00000000000..520515e9508 --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-results.ts @@ -0,0 +1,52 @@ +import { useEffect, useState } from 'react' +import type { DiffSearchRequest, DiffSearchResult } from './pierre-diff-search' + +export function usePierreDiffSearchResults(request: DiffSearchRequest | null) { + const [result, setResult] = useState<{ request: DiffSearchRequest; value: DiffSearchResult }>() + useEffect(() => { + if (!request || !request.query.text) { + return + } + let worker: Worker | undefined + let timeout: ReturnType | undefined + let disposed = false + const finish = (value: DiffSearchResult) => { + worker?.terminate() + clearTimeout(timeout) + if (!disposed) { + setResult({ request, value }) + } + } + const debounce = setTimeout(() => { + try { + worker = new Worker(new URL('./pierre-diff-search.worker.ts', import.meta.url), { + type: 'module' + }) + worker.onmessage = ({ data }: MessageEvent) => finish(data) + worker.onerror = () => + finish({ matches: [], truncated: false, error: 'Could not search this file' }) + worker.onmessageerror = () => + finish({ matches: [], truncated: false, error: 'Could not read search results' }) + timeout = setTimeout( + () => + finish({ + matches: [], + truncated: false, + error: 'Search took too long. Try a simpler expression.' + }), + 5_000 + ) + worker.postMessage(request) + } catch { + finish({ matches: [], truncated: false, error: 'Could not start search' }) + } + }, 120) + return () => { + disposed = true + clearTimeout(debounce) + clearTimeout(timeout) + worker?.terminate() + } + }, [request]) + return result?.request === request ? result.value : null +} diff --git a/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-view.ts b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-view.ts new file mode 100644 index 00000000000..cad3e2992ed --- /dev/null +++ b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-search-view.ts @@ -0,0 +1,96 @@ +import { useCallback, useLayoutEffect, useRef } from 'react' +import type { FileDiffMetadata, PostRenderPhase } from '@pierre/diffs' +import type { PierreDiffInstance } from './PierreDiffSurface' +import type { DiffSearchMatch } from './pierre-diff-search' +import type { DiffSearchSide } from './PierreDiffSearchBar' +import { + getPierreSearchRanges, + paintPierreSearchHighlights, + pierreSearchRevealLine +} from './pierre-diff-search-view' +import { scrollPierreDiffToLine } from './pierre-diff-scroll' + +export function usePierreDiffSearchView({ + fileDiff, + side, + matches, + active +}: { + fileDiff: FileDiffMetadata + side: DiffSearchSide + matches?: DiffSearchMatch[] + active?: DiffSearchMatch +}) { + const viewRef = useRef<{ host: HTMLElement; instance: PierreDiffInstance } | null>(null) + const owner = useRef({}) + const frame = useRef(null) + const pending = useRef(false) + const schedule = useCallback(() => { + if (frame.current !== null) { + return + } + frame.current = requestAnimationFrame(() => { + frame.current = null + const view = viewRef.current + if (!view || !matches) { + paintPierreSearchHighlights(owner.current) + return + } + const { host, instance } = view + if (active && pending.current) { + if ( + instance.revealLine(pierreSearchRevealLine(fileDiff, active.range.start.line + 1, side)) + ) { + return + } + scrollPierreDiffToLine({ + host, + container: host.closest('.scrollbar-editor'), + lineNumber: active.range.start.line + 1, + side, + linePosition: instance.getLinePosition?.(active.range.start.line + 1, side), + hunkIndex: 0, + hunkCount: 0 + }) + } + const ranges = getPierreSearchRanges(host, fileDiff, side, matches, active) + paintPierreSearchHighlights(owner.current, ranges) + if (pending.current && ranges.active.length) { + const range = ranges.active[0] + const row = range.startContainer.parentElement?.closest('[data-code]') + if (row) { + const rect = range.getBoundingClientRect(), + viewport = row.getBoundingClientRect() + if (rect.left < viewport.left || rect.right > viewport.right) { + row.scrollLeft += rect.left - viewport.left - row.clientWidth / 3 + } + } + pending.current = false + } + }) + }, [matches, active, fileDiff, side]) + useLayoutEffect(() => { + pending.current = Boolean(active) + schedule() + const token = owner.current + return () => { + if (frame.current !== null) { + cancelAnimationFrame(frame.current) + } + frame.current = null + paintPierreSearchHighlights(token) + } + }, [schedule, active]) + return useCallback( + (host: HTMLElement, phase: PostRenderPhase, instance: PierreDiffInstance) => { + if (phase === 'unmount') { + viewRef.current = null + paintPierreSearchHighlights(owner.current) + } else { + viewRef.current = { host, instance } + schedule() + } + }, + [schedule] + ) +} diff --git a/tests/e2e/diff-search-parity.spec.ts b/tests/e2e/diff-search-parity.spec.ts new file mode 100644 index 00000000000..f19b0068ac6 --- /dev/null +++ b/tests/e2e/diff-search-parity.spec.ts @@ -0,0 +1,152 @@ +import { readFileSync, rmSync, writeFileSync } from 'node:fs' +import { test, expect } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' +import { addAndActivateRepo } from './helpers/isolated-repo-activation' +import { createIsolatedLargeDiffRepo } from './large-diff-repro-fixtures' + +test.use({ seedTestRepo: false }) +for (const mode of ['editable-file', 'readonly-combined']) { + test(`finds both sides and preserves edit permissions in ${mode}`, async ({ + orcaPage, + registerPostElectronShutdownCleanup + }, testInfo) => { + const original = `export const oldOnly = 1\n${Array.from({ length: 599 }, (_, index) => `export const value${index} = ${index}\n`).join('')}` + const modified = `// inserted line\n${original.replace('oldOnly', 'newOnly')}` + const fixture = createIsolatedLargeDiffRepo(original) + registerPostElectronShutdownCleanup(async () => + rmSync(fixture.repoPath, { recursive: true, force: true }) + ) + writeFileSync(fixture.absolutePath, modified) + await waitForSessionReady(orcaPage) + await addAndActivateRepo(orcaPage, fixture.repoPath) + await orcaPage.evaluate( + (readonly) => + window.__store!.getState().updateSettings({ + diffDefaultView: readonly ? 'inline' : 'side-by-side', + diffWordWrap: false + }), + mode === 'readonly-combined' + ) + await orcaPage.getByRole('button', { name: /^Source Control/ }).click() + if (mode === 'readonly-combined') { + await orcaPage.getByRole('button', { name: 'Stage All', exact: true }).click() + await expect( + orcaPage.locator('[data-testid="source-control-entry"][data-source-control-area="staged"]') + ).toHaveCount(1) + await orcaPage.getByRole('button', { name: 'View all', exact: true }).first().click() + } else { + await orcaPage.locator('[data-testid="source-control-entry"]').first().click() + } + const host = orcaPage.locator('diffs-container').first() + const originalRow = host + .locator('[data-code] [data-line][data-line-type="change-deletion"]') + .first() + await originalRow.click({ timeout: 20_000 }) + await orcaPage.keyboard.press('ControlOrMeta+f') + const search = orcaPage.locator('[data-diff-search]') + const input = search.getByRole('textbox', { name: 'Find in diff', exact: true }) + await expect(input).toBeFocused() + await expect(search.getByRole('button', { name: 'Original', exact: true })).toHaveAttribute( + 'aria-pressed', + 'true' + ) + await input.fill('oldOnly') + await expect(search.getByRole('status')).toHaveText('1/1') + await expect(search.getByRole('button', { name: 'Toggle replace', exact: true })).toHaveCount(0) + await search.getByRole('button', { name: 'Use regular expression', exact: true }).click() + await input.fill('value44[89]') + await expect(search.getByRole('status')).toHaveText('1/2') + await input.press('F3') + await expect(search.getByRole('status')).toHaveText('2/2') + await input.press('Shift+F3') + await expect(search.getByRole('status')).toHaveText('1/2') + await search.getByRole('button', { name: 'Use regular expression', exact: true }).click() + await input.fill('value448') + await expect(search.getByRole('status')).toHaveText('1/1') + await expect( + host.locator('[data-code] [data-line]').filter({ hasText: 'value448 = 448' }).first() + ).toBeInViewport() + await orcaPage.screenshot({ path: testInfo.outputPath(`${mode}-original-search.png`) }) + await search.getByRole('button', { name: 'Modified', exact: true }).click() + await input.fill('newOnly') + await expect(search.getByRole('status')).toHaveText('1/1') + if (mode === 'readonly-combined') { + await expect(host.locator('[contenteditable="true"]')).toHaveCount(0) + await expect(search.getByRole('button', { name: 'Toggle replace', exact: true })).toHaveCount( + 0 + ) + const row = host + .locator('[data-code] [data-line][data-line-type="change-addition"]') + .filter({ hasText: 'newOnly' }) + await row.click() + await orcaPage.keyboard.type('CORRUPTED') + await expect(row).toHaveText('export const newOnly = 1\n') + expect(readFileSync(fixture.absolutePath, 'utf8')).toBe(modified) + } else { + await search.getByRole('button', { name: 'Use regular expression', exact: true }).click() + await input.fill('new(Only)') + await search.getByRole('button', { name: 'Toggle replace', exact: true }).click() + await search.getByRole('textbox', { name: 'Replace in diff', exact: true }).fill('changed$1') + await expect(search.getByRole('status')).toHaveText('1/1') + await search.getByRole('button', { name: 'Replace all', exact: true }).click() + const row = host + .locator('[data-code][data-additions] [data-line]') + .filter({ hasText: 'changedOnly' }) + await expect(row).toBeVisible() + await search.getByRole('button', { name: 'Close search', exact: true }).click() + await orcaPage.keyboard.press('ControlOrMeta+z') + await expect(host.locator('[data-code][data-additions]')).toContainText('newOnly') + await orcaPage.keyboard.press('ControlOrMeta+s') + await expect.poll(() => readFileSync(fixture.absolutePath, 'utf8')).toBe(modified) + } + }) +} + +test('cancels a pathological regex without blocking input', async ({ + orcaPage, + registerPostElectronShutdownCleanup +}) => { + const fixture = createIsolatedLargeDiffRepo('export const before = 1\n') + registerPostElectronShutdownCleanup(async () => + rmSync(fixture.repoPath, { recursive: true, force: true }) + ) + writeFileSync(fixture.absolutePath, `export const after = 1\n// ${'a'.repeat(80)}!\n`) + await waitForSessionReady(orcaPage) + await addAndActivateRepo(orcaPage, fixture.repoPath) + await orcaPage.getByRole('button', { name: /^Source Control/ }).click() + await orcaPage.locator('[data-testid="source-control-entry"]').first().click() + const host = orcaPage.locator('diffs-container').first() + await host + .locator('[data-code] [data-line][data-line-type="change-addition"]') + .first() + .click({ timeout: 20_000 }) + await orcaPage.keyboard.press('ControlOrMeta+f') + const search = orcaPage.locator('[data-diff-search]') + const input = search.getByRole('textbox', { name: 'Find in diff', exact: true }) + await search.getByRole('button', { name: 'Use regular expression', exact: true }).click() + await orcaPage.evaluate(() => { + let last = performance.now() + const probe = { maxLag: 0, timer: 0 } + probe.timer = window.setInterval(() => { + const now = performance.now() + probe.maxLag = Math.max(probe.maxLag, now - last - 20) + last = now + }, 20) + ;(window as unknown as { diffSearchProbe: typeof probe }).diffSearchProbe = probe + }) + await input.fill('(a+)+$') + await expect(search.getByRole('status')).toContainText('Search took too long', { timeout: 8_000 }) + await input.fill('(a+)+!$') + await expect(search.getByRole('status')).toHaveText('1/1', { timeout: 3_000 }) + await input.fill('(a+)+$') + await orcaPage.waitForTimeout(300) + await input.fill('after') + await expect(search.getByRole('status')).toHaveText('1/1', { timeout: 3_000 }) + const maxLag = await orcaPage.evaluate(() => { + const probe = (window as unknown as { diffSearchProbe: { timer: number; maxLag: number } }) + .diffSearchProbe + clearInterval(probe.timer) + return probe.maxLag + }) + expect(maxLag).toBeLessThan(1_000) +})