diff --git a/apps/docs/app/(diffs)/playground/PlaygroundClient.tsx b/apps/docs/app/(diffs)/playground/PlaygroundClient.tsx index 9c8340314..32c05ca43 100644 --- a/apps/docs/app/(diffs)/playground/PlaygroundClient.tsx +++ b/apps/docs/app/(diffs)/playground/PlaygroundClient.tsx @@ -38,6 +38,7 @@ import { } from '@pierre/icons'; import { useRouter, useSearchParams } from 'next/navigation'; import { useCallback, useEffect, useMemo, useState } from 'react'; +import { flushSync } from 'react-dom'; import { toast } from 'sonner'; import type { PlaygroundAnnotationMetadata } from './constants'; @@ -48,9 +49,25 @@ import { VIRTUALIZER_FILE_DIFFS, } from './constants'; import { PlaygroundCodeView } from './PlaygroundCodeView'; -import { CommentForm, ExampleThread } from './PlaygroundComments'; +import { + CommentForm, + CommentThread, + ExampleThread, +} from './PlaygroundComments'; import { PlaygroundVirtualizerElementView } from './PlaygroundVirtualizerElementView'; import { PlaygroundVirtualizerView } from './PlaygroundVirtualizerView'; +import type { + EditorMode, + HunkSeparatorValue, + LineHoverHighlight, + ViewMode, +} from './searchParams'; +import { + DARK_THEMES, + DEFAULTS, + LIGHT_THEMES, + parsePlaygroundSearchParams, +} from './searchParams'; import { useTheme } from '@/components/theme-provider'; import { Button } from '@/components/ui/button'; import { ButtonGroup, ButtonGroupItem } from '@/components/ui/button-group'; @@ -62,26 +79,6 @@ import { } from '@/components/ui/dropdown-menu'; import { Switch } from '@/components/ui/switch'; -const LIGHT_THEMES = [ - 'pierre-light', - 'pierre-light-soft', - 'catppuccin-latte', - 'github-light', - 'one-light', - 'solarized-light', -] as const; - -const DARK_THEMES = [ - 'pierre-dark', - 'pierre-dark-soft', - 'catppuccin-mocha', - 'dracula', - 'github-dark', - 'one-dark-pro', - 'tokyo-night', - 'vitesse-dark', -] as const; - const LINE_DIFF_OPTIONS = [ { value: 'word-alt', label: 'Word-Alt' }, { value: 'word', label: 'Word' }, @@ -96,8 +93,6 @@ const HUNK_SEPARATOR_OPTIONS = [ { value: 'metadata', label: 'Metadata' }, ] as const; -type HunkSeparatorValue = (typeof HUNK_SEPARATOR_OPTIONS)[number]['value']; - const LINE_HOVER_HIGHLIGHT_OPTIONS = [ { value: 'disabled', label: 'Disabled' }, { value: 'both', label: 'Line & number' }, @@ -105,20 +100,6 @@ const LINE_HOVER_HIGHLIGHT_OPTIONS = [ { value: 'line', label: 'Line' }, ] as const; -type LineHoverHighlight = - (typeof LINE_HOVER_HIGHLIGHT_OPTIONS)[number]['value']; - -// The editable surface is rendered read-only (Review) or attached to a live -// editor (Edit). Markers are diagnostics shown only while editing. -type EditorMode = 'review' | 'edit'; - -// The rendering surface the playground diff(s) are drawn with. 'normal' is the -// single editable FileDiff; 'virtualizer' renders several diffs with window -// scroll (vanilla Virtualizer); 'virtualizer-element' renders them with the -// React inside its own scroll region; 'codeview' renders a mix -// of diff/file items in CodeView's own scroller. -type ViewMode = 'normal' | 'virtualizer' | 'virtualizer-element' | 'codeview'; - const VIEW_MODE_OPTIONS = [ { value: 'normal', label: 'Normal' }, { value: 'virtualizer', label: 'Virtualizer (win)' }, @@ -149,28 +130,6 @@ type SharedRenderOptions = Pick< hunkSeparators: HunkSeparatorValue; }; -// Default values for URL param comparison -const DEFAULTS = { - viewMode: 'normal' as ViewMode, - diffStyle: 'split', - colorMode: 'system', - lightTheme: 'pierre-light', - darkTheme: 'pierre-dark', - diffIndicators: 'bars', - lineDiffType: 'word-alt', - lineHoverHighlight: 'disabled' as LineHoverHighlight, - hunkSeparators: 'line-info' as HunkSeparatorValue, - background: true, - lineNumbers: true, - wrap: true, - lineSelection: true, - gutterButton: true, - interactionMode: 'comment' as const, - annotations: true, - editorMode: 'review' as EditorMode, - markers: false, -} as const; - interface PlaygroundClientProps { prerenderedDiff: PreloadFileDiffResult; } @@ -688,138 +647,48 @@ export function PlaygroundClient({ prerenderedDiff }: PlaygroundClientProps) { // in sync with the rest of the app. See `effectiveColorMode`. const { resolvedColorScheme } = useTheme(); - const getParam = (key: string, defaultValue: T): T => { - return (searchParams.get(key) as T) ?? defaultValue; - }; - - const getBoolParam = (key: string, defaultValue: boolean): boolean => { - const value = searchParams.get(key); - if (value === null) return defaultValue; - return value === '1' || value === 'true'; - }; - - const getLineModeParam = (): 'select' | 'comment' | 'none' | null => { - const value = searchParams.get('lineMode'); - if (value === 'select' || value === 'comment' || value === 'none') { - return value; - } - return null; - }; - - const getLineHoverHighlightParam = (): LineHoverHighlight => { - const value = searchParams.get('hover'); - return ( - LINE_HOVER_HIGHLIGHT_OPTIONS.find((option) => option.value === value) - ?.value ?? DEFAULTS.lineHoverHighlight - ); - }; - - const [viewMode, setViewMode] = useState(() => { - const value = getParam('view', DEFAULTS.viewMode); - return value === 'virtualizer' || - value === 'virtualizer-element' || - value === 'codeview' - ? value - : 'normal'; - }); - - const [diffStyle, setDiffStyle] = useState<'split' | 'unified'>( - getParam('layout', DEFAULTS.diffStyle) as 'split' | 'unified' + // One-time parse of the querystring with the same parser the server used to + // build the prerendered payload, so the first client render agrees with the + // prerendered markup. + const [urlState] = useState(() => + parsePlaygroundSearchParams((key) => searchParams.get(key)) ); - const [colorMode, setColorMode] = useState<'system' | 'light' | 'dark'>( - getParam('mode', DEFAULTS.colorMode) as 'system' | 'light' | 'dark' + const [viewMode, setViewMode] = useState(urlState.viewMode); + const [diffStyle, setDiffStyle] = useState(urlState.diffStyle); + const [colorMode, setColorMode] = useState(urlState.colorMode); + const [selectedLightTheme, setSelectedLightTheme] = useState( + urlState.lightTheme ); - const [selectedLightTheme, setSelectedLightTheme] = useState< - (typeof LIGHT_THEMES)[number] - >(getParam('light', DEFAULTS.lightTheme) as (typeof LIGHT_THEMES)[number]); - const [selectedDarkTheme, setSelectedDarkTheme] = useState< - (typeof DARK_THEMES)[number] - >(getParam('dark', DEFAULTS.darkTheme) as (typeof DARK_THEMES)[number]); - - const [diffIndicators, setDiffIndicators] = useState( - getParam('indicators', DEFAULTS.diffIndicators) as DiffIndicators + const [selectedDarkTheme, setSelectedDarkTheme] = useState( + urlState.darkTheme ); - - const [lineDiffType, setLineDiffType] = useState< - 'word-alt' | 'word' | 'char' | 'none' - >( - getParam('inline', DEFAULTS.lineDiffType) as - | 'word-alt' - | 'word' - | 'char' - | 'none' + const [diffIndicators, setDiffIndicators] = useState(urlState.diffIndicators); + const [lineDiffType, setLineDiffType] = useState(urlState.lineDiffType); + const [lineHoverHighlight, setLineHoverHighlight] = useState( + urlState.lineHoverHighlight ); - - const [lineHoverHighlight, setLineHoverHighlight] = - useState(getLineHoverHighlightParam); - - const [hunkSeparators, setHunkSeparators] = useState( - getParam('hunks', DEFAULTS.hunkSeparators) - ); - + const [hunkSeparators, setHunkSeparators] = useState(urlState.hunkSeparators); const [disableBackground, setDisableBackground] = useState( - !getBoolParam('bg', DEFAULTS.background) + urlState.disableBackground ); const [disableLineNumbers, setDisableLineNumbers] = useState( - !getBoolParam('ln', DEFAULTS.lineNumbers) - ); - const [overflow, setOverflow] = useState<'wrap' | 'scroll'>( - getBoolParam('wrap', DEFAULTS.wrap) ? 'wrap' : 'scroll' + urlState.disableLineNumbers ); - - const initialLineMode = getLineModeParam(); + const [overflow, setOverflow] = useState(urlState.overflow); const [enableLineSelection, setEnableLineSelection] = useState( - initialLineMode === 'select' - ? true - : initialLineMode === 'comment' - ? false - : initialLineMode === 'none' - ? false - : getBoolParam('select', DEFAULTS.lineSelection) + urlState.enableLineSelection ); const [enableGutterUtility, setEnableGutterUtility] = useState( - initialLineMode === 'comment' - ? true - : initialLineMode === 'select' - ? false - : initialLineMode === 'none' - ? false - : getBoolParam('gutter', DEFAULTS.gutterButton) + urlState.enableGutterUtility ); const [showAnnotations, setShowAnnotations] = useState( - getBoolParam('annot', DEFAULTS.annotations) - ); - // Edit mode only exists in the Normal view (other views render per-file - // edit controls instead), so only honor `?edit=edit` when starting there. - const [editorMode, setEditorMode] = useState( - viewMode === 'normal' && getParam('edit', DEFAULTS.editorMode) === 'edit' - ? 'edit' - : 'review' + urlState.showAnnotations ); - const [showMarkers, setShowMarkers] = useState( - getBoolParam('markers', DEFAULTS.markers) - ); - - // Parse selected line range from URL - // Format: L15a (line 15 additions), L28-35a (lines 28-35 additions), L10d (line 10 deletions) - const parseLineSelection = (): SelectedLineRange | null => { - const lineParam = searchParams.get('line'); - if (lineParam == null) return null; - - const match = lineParam.match(/^(\d+)(?:-(\d+))?([ad])$/); - if (match == null) return null; - - const start = parseInt(match[1], 10); - const end = match[2] != null ? parseInt(match[2], 10) : start; - const side: 'additions' | 'deletions' = - match[3] === 'd' ? 'deletions' : 'additions'; - - return { start, end, side }; - }; - + const [editorMode, setEditorMode] = useState(urlState.editorMode); + const [showMarkers, setShowMarkers] = useState(urlState.showMarkers); const [selectedRange, setSelectedRange] = useState( - parseLineSelection + urlState.selectedRange ); // Keep URL updates at gesture boundaries instead of navigating on every // pointer move while the controlled selection follows a gutter drag. @@ -840,9 +709,29 @@ export function PlaygroundClient({ prerenderedDiff }: PlaygroundClientProps) { // The editor attaches to the diff's editable (new-file) side. Recreate it // when the diff layout or edit mode changes so it re-attaches to the freshly // relaid-out surface with a clean document instead of reusing a torn-down - // instance (mirrors LiveEditing's editor lifecycle). + // instance (mirrors LiveEditing's editor lifecycle). Edits remap annotation + // line numbers (an Enter above a comment shifts it down); onChange hands the + // remapped set back so the `lineAnnotations` prop — and the React-slotted + // comment content keyed by line number — follows the edit. The flushSync + // matters: the editor renamed the shadow-DOM annotation slots during this + // same keystroke, and until React commits the matching light-DOM `slot` + // attributes the comments project nowhere. A scheduled commit lands frames + // later (blank comments, collapsed rows); a synchronous one lands before + // this task's paint. // eslint-disable-next-line react-hooks/exhaustive-deps -- deps intentionally force a fresh editor; the factory takes no inputs - const editor = useMemo(() => new Editor({}), []); + const editor = useMemo( + () => + new Editor({ + onChange: (_file, lineAnnotations) => { + if (lineAnnotations != null) { + flushSync(() => { + setAnnotations(lineAnnotations); + }); + } + }, + }), + [] + ); // Apply (or clear) the demo markers whenever the editor, mode, or toggle // changes. `setMarkers` throws until the editor attaches to its surface @@ -1002,8 +891,27 @@ export function PlaygroundClient({ prerenderedDiff }: PlaygroundClientProps) { [] ); + // Submitting persists the form in place: the annotation keeps its position + // and gains the typed body, which flips its rendering to a comment thread. + const handleSubmitComment = useCallback( + (side: AnnotationSide | undefined, lineNumber: number, body: string) => { + setAnnotations((prev) => + prev.map((ann) => + ann.side === side && ann.lineNumber === lineNumber + ? { ...ann, metadata: { ...ann.metadata, body } } + : ann + ) + ); + setSelectedRange(null); + setCommittedSelectedRange(null); + }, + [] + ); + + // An open form is an annotation that is neither the seeded thread nor a + // submitted comment; it pauses the gutter utility so forms can't stack. const hasOpenCommentForm = annotations.some( - (ann) => ann.metadata.isThread !== true + (ann) => ann.metadata.isThread !== true && ann.metadata.body == null ); // The controls expose standalone selection and comments as separate modes. @@ -1032,6 +940,13 @@ export function PlaygroundClient({ prerenderedDiff }: PlaygroundClientProps) { if (mode !== 'normal') setEditorMode('review'); }, []); + const [usePrerenderedHTML, setUsePrerenderedHTML] = useState( + () => viewMode === 'normal' + ); + if (usePrerenderedHTML && viewMode !== 'normal') { + setUsePrerenderedHTML(false); + } + const controlsContentProps = { viewMode, setViewMode: setViewModeAndResetEditor, @@ -1143,6 +1058,9 @@ export function PlaygroundClient({ prerenderedDiff }: PlaygroundClientProps) { const fileDiff = ( { - addCommentAtRange(range); - } + ? addCommentAtRange : undefined, }} renderAnnotation={ showAnnotations ? (annotation) => annotation.metadata.isThread === true ? ( - + + handleCancelComment(annotation.side, annotation.lineNumber) + } + /> + ) : annotation.metadata.body != null ? ( + + handleCancelComment(annotation.side, annotation.lineNumber) + } + /> ) : ( ) : undefined diff --git a/apps/docs/app/(diffs)/playground/PlaygroundCodeView.tsx b/apps/docs/app/(diffs)/playground/PlaygroundCodeView.tsx index fe18116f6..9dd2c136c 100644 --- a/apps/docs/app/(diffs)/playground/PlaygroundCodeView.tsx +++ b/apps/docs/app/(diffs)/playground/PlaygroundCodeView.tsx @@ -15,9 +15,14 @@ import { import { Editor } from '@pierre/diffs/editor'; import { CodeView, useStableCallback } from '@pierre/diffs/react'; import { useCallback, useEffect, useMemo, useState } from 'react'; +import { flushSync } from 'react-dom'; import type { PlaygroundAnnotationMetadata } from './constants'; -import { CommentForm, ExampleThread } from './PlaygroundComments'; +import { + CommentForm, + CommentThread, + ExampleThread, +} from './PlaygroundComments'; const CODE_VIEW_STYLES = { height: '70vh', overflow: 'auto' } as const; @@ -45,7 +50,8 @@ interface PlaygroundCodeViewProps { // items re-diff the edited new side against the original old side. // // Annotations ride on item data: a gutter utility gesture appends a comment -// form at its final line, and cancelling the form removes it again. +// form at its final line, submitting persists it as a comment thread, and +// cancelling removes it again. export function PlaygroundCodeView({ items: initialItems, options, @@ -67,6 +73,46 @@ export function PlaygroundCodeView({ ); }, []); + // Edits remap annotation line numbers (an Enter above a comment shifts it + // down); this writes the remapped set back to the owning item, with the + // version bump every item-data change requires — CodeView drops + // same-version pushes, and its render loop re-applies `item.annotations` + // to the instance on every pass, so a stale item would snap the comment + // back to its pre-edit line. flushSync commits in the same task as the + // editor's shadow-slot rename (the items push is a layout effect), so the + // comment is never projected nowhere between frames. The identity bail + // keeps ordinary typing free: the editor passes the same array reference + // when nothing remapped. + const handleEditChange = useCallback( + ( + item: PlaygroundItem, + _file: FileContents, + lineAnnotations?: DiffLineAnnotation[] + ) => { + if (lineAnnotations == null) { + return; + } + flushSync(() => { + setItems((current) => { + const target = current.find((existing) => existing.id === item.id); + if (target == null || target.annotations === lineAnnotations) { + return current; + } + return current.map((existing) => + existing.id === item.id + ? { + ...existing, + annotations: lineAnnotations, + version: (existing.version ?? 0) + 1, + } + : existing + ); + }); + }); + }, + [] + ); + // Committing a finished edit session is user-space: CodeView only ends the // session and reports the final contents through this lifecycle. The app // commits with one combined item write — the new file/fileDiff (fresh @@ -193,6 +239,48 @@ export function PlaygroundCodeView({ [] ); + // Submitting persists the form in place: the annotation keeps its position + // and gains the typed body, which flips its rendering to a comment thread. + const submitCommentAtLine = useCallback( + ( + itemId: string, + side: AnnotationSide | undefined, + lineNumber: number, + body: string + ) => { + setItems((current) => + current.map((item) => { + if (item.id !== itemId) { + return item; + } + const version = (item.version ?? 0) + 1; + if (item.type === 'file') { + return { + ...item, + annotations: (item.annotations ?? []).map((a) => + a.lineNumber === lineNumber + ? { ...a, metadata: { ...a.metadata, body } } + : a + ), + version, + }; + } + return { + ...item, + annotations: (item.annotations ?? []).map((a) => + a.side === side && a.lineNumber === lineNumber + ? { ...a, metadata: { ...a.metadata, body } } + : a + ), + version, + }; + }) + ); + setSelectedLines(null); + }, + [] + ); + // Annotations live on item data, so hiding them is a data change: turning // the toggle off clears any comments that were added. useEffect(() => { @@ -209,12 +297,15 @@ export function PlaygroundCodeView({ ); }, [showAnnotations]); - // Match the Normal view's precedence: an open comment form pauses the - // gutter utility so the form can't stack. + // Match the Normal view's precedence: an open comment form (neither a + // thread nor a submitted comment) pauses the gutter utility so forms can't + // stack. const hasOpenCommentForm = items.some( (item) => item.annotations?.some( - (annotation) => annotation.metadata.isThread !== true + (annotation) => + annotation.metadata.isThread !== true && + annotation.metadata.body == null ) === true ); const canSelectLines = @@ -243,16 +334,36 @@ export function PlaygroundCodeView({ | DiffLineAnnotation, item: PlaygroundItem ) => { + const side = 'side' in annotation ? annotation.side : undefined; if (annotation.metadata.isThread === true) { - return ; + return ( + + removeCommentAtLine(item.id, side, annotation.lineNumber) + } + /> + ); + } + if (annotation.metadata.body != null) { + return ( + + removeCommentAtLine(item.id, side, annotation.lineNumber) + } + /> + ); } return ( removeCommentAtLine(item.id, side, lineNumber) } + onSubmit={(side, lineNumber, body) => + submitCommentAtLine(item.id, side, lineNumber, body) + } /> ); } @@ -281,6 +392,7 @@ export function PlaygroundCodeView({ selectedLines={selectedLines} onSelectedLinesChange={setSelectedLines} createEditor={createEditor} + onItemEditChange={handleEditChange} onItemEditComplete={handleEditComplete} renderHeaderMetadata={renderHeaderMetadata} renderAnnotation={renderAnnotation} diff --git a/apps/docs/app/(diffs)/playground/PlaygroundComments.tsx b/apps/docs/app/(diffs)/playground/PlaygroundComments.tsx index 897081daf..859c2516c 100644 --- a/apps/docs/app/(diffs)/playground/PlaygroundComments.tsx +++ b/apps/docs/app/(diffs)/playground/PlaygroundComments.tsx @@ -7,16 +7,23 @@ import { Avatar, AvatarFallback, AvatarImage } from '@/components/ui/avatar'; import { Button } from '@/components/ui/button'; // Annotation content shared by the playground's view modes: a comment form -// for freshly added gutter comments and a static example thread. `side` is -// undefined for plain file annotations, which have no diff side. +// for freshly added gutter comments, the submitted comment it becomes, and a +// static example thread. `side` is undefined for plain file annotations, +// which have no diff side. export function CommentForm({ side, lineNumber, onCancel, + onSubmit, }: { side: AnnotationSide | undefined; lineNumber: number; onCancel: (side: AnnotationSide | undefined, lineNumber: number) => void; + onSubmit: ( + side: AnnotationSide | undefined, + lineNumber: number, + body: string + ) => void; }) { const textareaRef = useRef(null); @@ -30,6 +37,16 @@ export function CommentForm({ onCancel(side, lineNumber); }, [side, lineNumber, onCancel]); + const handleSubmit = useCallback(() => { + const body = textareaRef.current?.value.trim() ?? ''; + // An empty submit has no comment to keep; treat it as a cancel. + if (body === '') { + onCancel(side, lineNumber); + return; + } + onSubmit(side, lineNumber, body); + }, [side, lineNumber, onCancel, onSubmit]); + return (
{ - console.log('Comment submitted at', side, lineNumber); - handleCancel(); - }} + onClick={handleSubmit} > Comment @@ -94,7 +108,59 @@ export function CommentForm({ ); } -export function ExampleThread() { +// The persisted form of a submitted CommentForm: a single-message thread +// showing the typed text. Delete removes the owning annotation. +export function CommentThread({ + body, + onDelete, +}: { + body: string; + onDelete: () => void; +}) { + return ( +
+
+
+
+ + + Y + +
+
+
+ You + just now +
+

+ {body} +

+
+
+ +
+ +
+
+
+ ); +} + +// Delete is optional so the thread can render in read-only contexts; when +// provided it removes the owning annotation. +export function ExampleThread({ onDelete }: { onDelete?: () => void }) { return (
Resolve + {onDelete != null && ( + + )}
diff --git a/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerElementView.tsx b/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerElementView.tsx index a37ee6e7c..8ba46473f 100644 --- a/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerElementView.tsx +++ b/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerElementView.tsx @@ -1,6 +1,7 @@ 'use client'; import type { + AnnotationSide, DiffLineAnnotation, FileDiffMetadata, FileDiffOptions, @@ -9,15 +10,17 @@ import type { import { Editor } from '@pierre/diffs/editor'; import { EditProvider, FileDiff, Virtualizer } from '@pierre/diffs/react'; import { useCallback, useEffect, useMemo, useState } from 'react'; +import { flushSync } from 'react-dom'; +import type { PlaygroundAnnotationMetadata } from './constants'; import { ITEM_UNSAFE_CSS } from './constants'; -import { CommentForm } from './PlaygroundComments'; +import { CommentForm, CommentThread } from './PlaygroundComments'; const SCROLL_REGION_STYLES = { height: '70vh', overflow: 'auto' } as const; interface PlaygroundVirtualizerElementViewProps { diffs: FileDiffMetadata[]; - options: FileDiffOptions; + options: FileDiffOptions; enableLineSelection: boolean; enableGutterComments: boolean; showAnnotations: boolean; @@ -57,7 +60,7 @@ export function PlaygroundVirtualizerElementView({ interface ElementVirtualizerDiffProps { fileDiff: FileDiffMetadata; - options: FileDiffOptions; + options: FileDiffOptions; enableLineSelection: boolean; enableGutterComments: boolean; showAnnotations: boolean; @@ -75,15 +78,34 @@ function ElementVirtualizerDiff({ enableGutterComments, showAnnotations, }: ElementVirtualizerDiffProps) { - const editor = useMemo(() => new Editor({}), []); const [editing, setEditing] = useState(false); const [annotations, setAnnotations] = useState< - DiffLineAnnotation[] + DiffLineAnnotation[] >([]); const [selectedLines, setSelectedLines] = useState( null ); + // Edits remap annotation line numbers; onChange hands the remapped set back + // so the `lineAnnotations` prop — and the React-slotted comment content + // keyed by line number — follows the edit. The flushSync matters: the + // editor renamed the shadow-DOM annotation slots during this same + // keystroke, and a scheduled commit would leave the comments projected + // nowhere for the frames in between. + const editor = useMemo( + () => + new Editor({ + onChange: (_file, lineAnnotations) => { + if (lineAnnotations != null) { + flushSync(() => { + setAnnotations(lineAnnotations); + }); + } + }, + }), + [] + ); + const addCommentAtRange = useCallback((range: SelectedLineRange) => { const side = range.endSide ?? range.side; if (side == null) { @@ -96,15 +118,19 @@ function ElementVirtualizerDiff({ annotation.side === side && annotation.lineNumber === lineNumber ) ? current - : [...current, { side, lineNumber }] + : [ + ...current, + { + side, + lineNumber, + metadata: { key: `${side}-${lineNumber}`, isThread: false }, + }, + ] ); }, []); const removeCommentAtLine = useCallback( - ( - side: DiffLineAnnotation['side'] | undefined, - lineNumber: number - ) => { + (side: AnnotationSide | undefined, lineNumber: number) => { setAnnotations((current) => current.filter( (annotation) => @@ -116,21 +142,42 @@ function ElementVirtualizerDiff({ [] ); + // Submitting persists the form in place: the annotation keeps its position + // and gains the typed body, which flips its rendering to a comment thread. + const submitCommentAtLine = useCallback( + (side: AnnotationSide | undefined, lineNumber: number, body: string) => { + setAnnotations((current) => + current.map((annotation) => + annotation.side === side && annotation.lineNumber === lineNumber + ? { ...annotation, metadata: { ...annotation.metadata, body } } + : annotation + ) + ); + setSelectedLines(null); + }, + [] + ); + useEffect(() => { if (!showAnnotations) { setSelectedLines(null); } }, [showAnnotations]); - // Match the other views' precedence: an open comment form pauses the gutter - // utility so another form cannot be opened beneath it. - const hasOpenCommentForm = annotations.length > 0; + // Match the other views' precedence: an open comment form (no submitted + // body yet) pauses the gutter utility so another form cannot be opened + // beneath it. + const hasOpenCommentForm = annotations.some( + (annotation) => annotation.metadata.body == null + ); const canSelectLines = enableLineSelection && !enableGutterComments && !hasOpenCommentForm; const canUseGutterComments = enableGutterComments && showAnnotations && !hasOpenCommentForm; - const fileDiffOptions = useMemo>( + const fileDiffOptions = useMemo< + FileDiffOptions + >( () => ({ ...options, stickyHeader: true, @@ -166,13 +213,23 @@ function ElementVirtualizerDiff({ Edit )} - renderAnnotation={(annotation) => ( - - )} + renderAnnotation={(annotation) => + annotation.metadata.body != null ? ( + + removeCommentAtLine(annotation.side, annotation.lineNumber) + } + /> + ) : ( + + ) + } /> ); diff --git a/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerView.tsx b/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerView.tsx index 9d5b41625..ae77bed99 100644 --- a/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerView.tsx +++ b/apps/docs/app/(diffs)/playground/PlaygroundVirtualizerView.tsx @@ -10,14 +10,15 @@ import { import { Editor } from '@pierre/diffs/editor'; import { useWorkerPool } from '@pierre/diffs/react'; import { useEffect, useRef } from 'react'; +import { flushSync } from 'react-dom'; import { createRoot, type Root } from 'react-dom/client'; import { ITEM_UNSAFE_CSS } from './constants'; -import { CommentForm } from './PlaygroundComments'; +import { CommentForm, CommentThread } from './PlaygroundComments'; interface PlaygroundVirtualizerViewProps { diffs: FileDiffMetadata[]; - options: FileDiffOptions; + options: FileDiffOptions; enableLineSelection: boolean; enableGutterComments: boolean; showAnnotations: boolean; @@ -53,11 +54,24 @@ const VIRTUALIZER_CUSTOM_CSS = `${ITEM_UNSAFE_CSS} } `; +// Annotations carry a stable key in metadata so their React roots survive +// edit-session remaps: line numbers move when lines are inserted above a +// comment, but the metadata reference rides through the editor's remap +// untouched. +interface VirtualizerAnnotationMetadata { + key: string; + /** Submitted comment text. Present once the comment form was submitted; + * absent while the form is still open. */ + body?: string; +} + +type VirtualizerAnnotation = DiffLineAnnotation; + function annotationKey( index: number, - annotation: DiffLineAnnotation + annotation: VirtualizerAnnotation ): string { - return `${index}:${annotation.side}:${annotation.lineNumber}`; + return `${index}:${annotation.metadata.key}`; } // The "Virtualizer (window)" mode: renders a list of full diffs through the @@ -86,9 +100,12 @@ export function PlaygroundVirtualizerView({ }: PlaygroundVirtualizerViewProps) { const pool = useWorkerPool(); const contentRef = useRef(null); - const instancesRef = useRef([]); - const annotationsRef = useRef[][]>([]); + const instancesRef = useRef< + VirtualizedFileDiff[] + >([]); + const annotationsRef = useRef([]); const annotationRootsRef = useRef(new Map()); + const annotationKeyCounterRef = useRef(0); // React forbids synchronously unmounting a root from inside its own event // handler (the Cancel button lives in the root being removed), so unmounts @@ -117,7 +134,7 @@ export function PlaygroundVirtualizerView({ virtualizer.setup(document); annotationsRef.current = diffs.map(() => []); - const editors: Editor[] = []; + const editors: Editor[] = []; const instances = diffs.map((fileDiff, index) => { // `diffs-container` is the library's default (registered) container // element. We create and append it ourselves so the virtualizer can @@ -126,7 +143,34 @@ export function PlaygroundVirtualizerView({ fileContainer.style.display = 'block'; content.appendChild(fileContainer); - const editor = new Editor({}); + // Edits remap annotation line numbers; onChange hands the remapped set + // back so this view's annotation source of truth follows the edit — + // otherwise the next host-driven render snaps comments back to their + // pre-edit lines. An annotation whose line was deleted is dropped from + // the set; retire its orphaned React root. + const editor = new Editor({ + onChange: (_file, lineAnnotations) => { + if (lineAnnotations == null) { + return; + } + const previous = annotationsRef.current[index]; + if (previous === lineAnnotations) { + return; + } + annotationsRef.current[index] = lineAnnotations; + const liveKeys = new Set( + lineAnnotations.map((annotation) => + annotationKey(index, annotation) + ) + ); + for (const annotation of previous) { + const key = annotationKey(index, annotation); + if (!liveKeys.has(key)) { + unmountAnnotationRoot(key); + } + } + }, + }); editors.push(editor); const { element: editToggle, input } = createEditToggle(); @@ -137,67 +181,105 @@ export function PlaygroundVirtualizerView({ }); }; - const removeAnnotation = (annotation: DiffLineAnnotation) => { + const removeAnnotation = (annotation: VirtualizerAnnotation) => { annotationsRef.current[index] = annotationsRef.current[index].filter( - (existing) => - !( - existing.side === annotation.side && - existing.lineNumber === annotation.lineNumber - ) + (existing) => existing.metadata.key !== annotation.metadata.key ); instance.setSelectedLines(null); rerenderWithAnnotations(); unmountAnnotationRoot(annotationKey(index, annotation)); }; - const instance: VirtualizedFileDiff = new VirtualizedFileDiff( - { - ...options, - renderHeaderMetadata: () => editToggle, - stickyHeader: true, - unsafeCSS: VIRTUALIZER_CUSTOM_CSS, - enableLineSelection: enableLineSelection && !enableGutterComments, - enableGutterUtility: enableGutterComments && showAnnotations, - onGutterUtilityClick: (range) => { - const side = range.endSide ?? range.side; - if (side == null) { - return; - } - const lineNumber = range.end; - const annotations = annotationsRef.current[index]; - if ( - annotations.some( - (annotation) => - annotation.side === side && - annotation.lineNumber === lineNumber - ) - ) { - return; - } - annotations.push({ side, lineNumber }); - rerenderWithAnnotations(); - }, - renderAnnotation: (annotation) => { - const container = document.createElement('div'); - const root = createRoot(container); - annotationRootsRef.current.set( - annotationKey(index, annotation), - root - ); - root.render( - removeAnnotation(annotation)} - /> - ); - return container; + // Submitting persists the form in place: the annotation keeps its key + // and position and gains the typed body, which flips its rendering to a + // comment thread. The fresh metadata object fails the diff's annotation + // equality check, so the re-render rebuilds the wrapper and + // renderAnnotation swaps the root's content. + const submitAnnotation = ( + annotation: VirtualizerAnnotation, + body: string + ) => { + annotationsRef.current[index] = annotationsRef.current[index].map( + (existing) => + existing.metadata.key === annotation.metadata.key + ? { ...existing, metadata: { ...existing.metadata, body } } + : existing + ); + instance.setSelectedLines(null); + rerenderWithAnnotations(); + }; + + const instance: VirtualizedFileDiff = + new VirtualizedFileDiff( + { + ...options, + renderHeaderMetadata: () => editToggle, + stickyHeader: true, + unsafeCSS: VIRTUALIZER_CUSTOM_CSS, + enableLineSelection: enableLineSelection && !enableGutterComments, + enableGutterUtility: enableGutterComments && showAnnotations, + onGutterUtilityClick: (range) => { + const side = range.endSide ?? range.side; + if (side == null) { + return; + } + const lineNumber = range.end; + const annotations = annotationsRef.current[index]; + if ( + annotations.some( + (annotation) => + annotation.side === side && + annotation.lineNumber === lineNumber + ) + ) { + return; + } + annotations.push({ + side, + lineNumber, + metadata: { + key: `comment-${annotationKeyCounterRef.current++}`, + }, + }); + rerenderWithAnnotations(); + }, + renderAnnotation: (annotation) => { + // A remap re-renders the same annotation under the same key; the + // previous wrapper (and the root inside it) is being discarded by + // the diff, so retire that root before mounting the replacement. + const key = annotationKey(index, annotation); + unmountAnnotationRoot(key); + const container = document.createElement('div'); + const root = createRoot(container); + annotationRootsRef.current.set(key, root); + // Render synchronously so the recreated comment paints in the + // same frame as the row that hosts it. + flushSync(() => { + root.render( + annotation.metadata.body != null ? ( + removeAnnotation(annotation)} + /> + ) : ( + removeAnnotation(annotation)} + onSubmit={(_side, _lineNumber, body) => + submitAnnotation(annotation, body) + } + /> + ) + ); + }); + return container; + }, }, - }, - virtualizer, - undefined, - pool - ); + virtualizer, + undefined, + pool + ); // Attaching the editor flips the new-file surface to contentEditable; // detaching restores read-only review. diff --git a/apps/docs/app/(diffs)/playground/constants.ts b/apps/docs/app/(diffs)/playground/constants.ts index d2f1d14d7..629833cd4 100644 --- a/apps/docs/app/(diffs)/playground/constants.ts +++ b/apps/docs/app/(diffs)/playground/constants.ts @@ -5,11 +5,13 @@ import { } from '@pierre/diffs'; import type { PreloadFileDiffOptions } from '@pierre/diffs/ssr'; +import type { PlaygroundUrlState } from './searchParams'; import { CustomScrollbarCSS } from '@/components/CustomScrollbarCSS'; export interface PlaygroundAnnotationMetadata { key: string; isThread: boolean; + body?: string; } // Multi-hunk diff: edits at top, middle (annotation on new line 25), and @@ -148,40 +150,55 @@ export const PLAYGROUND_MARKERS = [ }, ]; -export const PLAYGROUND_DIFF: PreloadFileDiffOptions = +const PLAYGROUND_FILE_DIFF = parseDiffFromFile( { - fileDiff: parseDiffFromFile( - { - name: 'api/users.ts', - contents: OLD_USERS_CONTENT, - }, - { - name: 'api/users.ts', - contents: NEW_USERS_CONTENT, - } - ), - // Match the client's default render (PlaygroundClient DEFAULTS): ship both - // light and dark themes with themeType 'system' so the prerendered shadow - // DOM resolves via the native CSS `light-dark()` against the pre-paint - // color-scheme. A single fixed theme would force one color-scheme server - // side and flash when the client re-resolves to the other on first paint. + name: 'api/users.ts', + contents: OLD_USERS_CONTENT, + }, + { + name: 'api/users.ts', + contents: NEW_USERS_CONTENT, + } +); + +const PLAYGROUND_ANNOTATIONS = [ + { + side: 'additions', + lineNumber: 25, + metadata: { + key: 'additions-25', + isThread: true, + }, + }, +] satisfies PreloadFileDiffOptions['annotations']; + +// Maps the shared URL state onto the preload options, so the prerendered +// markup matches what the client derives from the same querystring — the +// markup paints before hydration, and a drifted option would show the +// server's presentation until the first client repaint. `colorMode` maps to +// themeType directly: 'system' ships both themes and resolves via the native +// CSS `light-dark()` against the pre-paint color-scheme, so no flash when +// the client theme controller settles. +export function getPlaygroundPreloadOptions( + state: PlaygroundUrlState +): PreloadFileDiffOptions { + return { + fileDiff: PLAYGROUND_FILE_DIFF, options: { - theme: { dark: 'pierre-dark', light: 'pierre-light' }, - themeType: 'system', - diffStyle: 'split', + theme: { dark: state.darkTheme, light: state.lightTheme }, + themeType: state.colorMode, + diffStyle: state.diffStyle, + overflow: state.overflow, + diffIndicators: state.diffIndicators, + lineDiffType: state.lineDiffType, + hunkSeparators: state.hunkSeparators, + disableBackground: state.disableBackground, + disableLineNumbers: state.disableLineNumbers, unsafeCSS: CustomScrollbarCSS, }, - annotations: [ - { - side: 'additions', - lineNumber: 25, - metadata: { - key: 'additions-25', - isThread: true, - }, - }, - ], + annotations: state.showAnnotations ? PLAYGROUND_ANNOTATIONS : [], }; +} // ----------------------------------------------------------------------------- // Multi-item fixtures for the Virtualizer and CodeView playground modes. diff --git a/apps/docs/app/(diffs)/playground/page.tsx b/apps/docs/app/(diffs)/playground/page.tsx index f1af6a5e6..7f06768b3 100644 --- a/apps/docs/app/(diffs)/playground/page.tsx +++ b/apps/docs/app/(diffs)/playground/page.tsx @@ -2,13 +2,30 @@ import { preloadFileDiff } from '@pierre/diffs/ssr'; import { Suspense } from 'react'; import { WorkerPoolContext } from '../_components/WorkerPoolContext'; -import { PLAYGROUND_DIFF } from './constants'; +import { getPlaygroundPreloadOptions } from './constants'; import { PlaygroundClient } from './PlaygroundClient'; +import { parsePlaygroundSearchParams } from './searchParams'; import Footer from '@/components/Footer'; import { Header } from '@/components/Header'; -export default async function PlaygroundPage() { - const prerenderedDiff = await preloadFileDiff(PLAYGROUND_DIFF); +type PlaygroundSearchParams = Record; + +export default async function PlaygroundPage({ + searchParams, +}: { + searchParams?: Promise | PlaygroundSearchParams; +}) { + const params = (await searchParams) ?? {}; + // Server and client parse the querystring with the same parser, so the + // prerendered markup matches the client's first render for any + // parameterized load, not just the defaults. + const urlState = parsePlaygroundSearchParams((key) => { + const value = params[key]; + return (Array.isArray(value) ? value[0] : value) ?? null; + }); + const prerenderedDiff = await preloadFileDiff( + getPlaygroundPreloadOptions(urlState) + ); return (
diff --git a/apps/docs/app/(diffs)/playground/searchParams.ts b/apps/docs/app/(diffs)/playground/searchParams.ts new file mode 100644 index 000000000..125ff7787 --- /dev/null +++ b/apps/docs/app/(diffs)/playground/searchParams.ts @@ -0,0 +1,189 @@ +import type { DiffIndicators, SelectedLineRange } from '@pierre/diffs'; + +// The playground's URL state, parsed identically on the server (to build the +// prerendered payload) and on the client (to seed component state). The +// prerendered markup paints before hydration, so any parameter the two sides +// derived differently would show the server's presentation until the first +// client repaint — one parser, consumed by both, removes that class of drift. + +export const LIGHT_THEMES = [ + 'pierre-light', + 'pierre-light-soft', + 'catppuccin-latte', + 'github-light', + 'one-light', + 'solarized-light', +] as const; + +export const DARK_THEMES = [ + 'pierre-dark', + 'pierre-dark-soft', + 'catppuccin-mocha', + 'dracula', + 'github-dark', + 'one-dark-pro', + 'tokyo-night', + 'vitesse-dark', +] as const; + +export type PlaygroundLightTheme = (typeof LIGHT_THEMES)[number]; +export type PlaygroundDarkTheme = (typeof DARK_THEMES)[number]; + +const VIEW_MODES = [ + 'normal', + 'virtualizer', + 'virtualizer-element', + 'codeview', +] as const; + +const DIFF_STYLES = ['split', 'unified'] as const; +const COLOR_MODES = ['system', 'light', 'dark'] as const; +const DIFF_INDICATORS = ['bars', 'classic', 'none'] as const; +const LINE_DIFF_TYPES = ['word-alt', 'word', 'char', 'none'] as const; +const HUNK_SEPARATOR_VALUES = [ + 'line-info', + 'line-info-basic', + 'simple', + 'metadata', +] as const; +const LINE_HOVER_HIGHLIGHTS = ['disabled', 'both', 'number', 'line'] as const; +const LINE_MODES = ['select', 'comment', 'none'] as const; + +// The rendering surface the playground diff(s) are drawn with. 'normal' is the +// single editable FileDiff; 'virtualizer' renders several diffs with window +// scroll (vanilla Virtualizer); 'virtualizer-element' renders them with the +// React inside its own scroll region; 'codeview' renders a mix +// of diff/file items in CodeView's own scroller. +export type ViewMode = (typeof VIEW_MODES)[number]; + +// The editable surface is rendered read-only (Review) or attached to a live +// editor (Edit). Markers are diagnostics shown only while editing. +export type EditorMode = 'review' | 'edit'; + +export type HunkSeparatorValue = (typeof HUNK_SEPARATOR_VALUES)[number]; +export type LineHoverHighlight = (typeof LINE_HOVER_HIGHLIGHTS)[number]; +export type PlaygroundLineDiffType = (typeof LINE_DIFF_TYPES)[number]; + +// Default values for URL param comparison +export const DEFAULTS = { + viewMode: 'normal' as ViewMode, + diffStyle: 'split', + colorMode: 'system', + lightTheme: 'pierre-light', + darkTheme: 'pierre-dark', + diffIndicators: 'bars', + lineDiffType: 'word-alt', + lineHoverHighlight: 'disabled' as LineHoverHighlight, + hunkSeparators: 'line-info' as HunkSeparatorValue, + background: true, + lineNumbers: true, + wrap: true, + lineSelection: true, + gutterButton: true, + interactionMode: 'comment' as const, + annotations: true, + editorMode: 'review' as EditorMode, + markers: false, +} as const; + +export interface PlaygroundUrlState { + viewMode: ViewMode; + diffStyle: (typeof DIFF_STYLES)[number]; + colorMode: (typeof COLOR_MODES)[number]; + lightTheme: PlaygroundLightTheme; + darkTheme: PlaygroundDarkTheme; + diffIndicators: DiffIndicators; + lineDiffType: PlaygroundLineDiffType; + lineHoverHighlight: LineHoverHighlight; + hunkSeparators: HunkSeparatorValue; + disableBackground: boolean; + disableLineNumbers: boolean; + overflow: 'wrap' | 'scroll'; + enableLineSelection: boolean; + enableGutterUtility: boolean; + showAnnotations: boolean; + editorMode: EditorMode; + showMarkers: boolean; + selectedRange: SelectedLineRange | null; +} + +// Narrows a raw param to one of the allowed values, falling back for absent +// or unrecognized input, so garbage in the URL degrades to the default +// instead of leaking into render options. +function pick( + value: string | null, + allowed: readonly T[], + fallback: T +): T { + return allowed.find((option) => option === value) ?? fallback; +} + +function pickBool(value: string | null, fallback: boolean): boolean { + if (value === null) return fallback; + return value === '1' || value === 'true'; +} + +// Selected line range format: L15a (line 15 additions), L28-35a (lines 28-35 +// additions), L10d (line 10 deletions). +function parseLineSelection(value: string | null): SelectedLineRange | null { + if (value == null) return null; + const match = value.match(/^(\d+)(?:-(\d+))?([ad])$/); + if (match == null) return null; + const start = parseInt(match[1], 10); + const end = match[2] != null ? parseInt(match[2], 10) : start; + const side: 'additions' | 'deletions' = + match[3] === 'd' ? 'deletions' : 'additions'; + return { start, end, side }; +} + +export function parsePlaygroundSearchParams( + get: (key: string) => string | null +): PlaygroundUrlState { + const viewMode = pick(get('view'), VIEW_MODES, DEFAULTS.viewMode); + + // `lineMode` is the combined interaction switch; when absent, the separate + // legacy `select`/`gutter` booleans apply. + const lineModeParam = get('lineMode'); + const lineMode = + LINE_MODES.find((option) => option === lineModeParam) ?? null; + const enableLineSelection = + lineMode != null + ? lineMode === 'select' + : pickBool(get('select'), DEFAULTS.lineSelection); + const enableGutterUtility = + lineMode != null + ? lineMode === 'comment' + : pickBool(get('gutter'), DEFAULTS.gutterButton); + + return { + viewMode, + diffStyle: pick(get('layout'), DIFF_STYLES, 'split'), + colorMode: pick(get('mode'), COLOR_MODES, 'system'), + lightTheme: pick(get('light'), LIGHT_THEMES, DEFAULTS.lightTheme), + darkTheme: pick(get('dark'), DARK_THEMES, DEFAULTS.darkTheme), + diffIndicators: pick(get('indicators'), DIFF_INDICATORS, 'bars'), + lineDiffType: pick(get('inline'), LINE_DIFF_TYPES, 'word-alt'), + lineHoverHighlight: pick( + get('hover'), + LINE_HOVER_HIGHLIGHTS, + DEFAULTS.lineHoverHighlight + ), + hunkSeparators: pick( + get('hunks'), + HUNK_SEPARATOR_VALUES, + DEFAULTS.hunkSeparators + ), + disableBackground: !pickBool(get('bg'), DEFAULTS.background), + disableLineNumbers: !pickBool(get('ln'), DEFAULTS.lineNumbers), + overflow: pickBool(get('wrap'), DEFAULTS.wrap) ? 'wrap' : 'scroll', + enableLineSelection, + enableGutterUtility, + showAnnotations: pickBool(get('annot'), DEFAULTS.annotations), + // Edit mode only exists in the Normal view (other views render per-file + // edit controls instead), so only honor `?edit=edit` when starting there. + editorMode: + viewMode === 'normal' && get('edit') === 'edit' ? 'edit' : 'review', + showMarkers: pickBool(get('markers'), DEFAULTS.markers), + selectedRange: parseLineSelection(get('line')), + }; +} diff --git a/packages/diffs/src/components/FileDiff.ts b/packages/diffs/src/components/FileDiff.ts index dcdfb8d13..7a0cdb8f4 100644 --- a/packages/diffs/src/components/FileDiff.ts +++ b/packages/diffs/src/components/FileDiff.ts @@ -82,6 +82,7 @@ import { } from '../utils/editSessionHunks'; import { getDiffFileInput } from '../utils/getDiffFileInput'; import { getDiffHunksRendererOptions } from '../utils/getDiffHunksRendererOptions'; +import { getHunkSideStartBoundary } from '../utils/getHunkSideBoundaries'; import { getLineAnnotationName } from '../utils/getLineAnnotationName'; import { getOrCreateCodeNode } from '../utils/getOrCreateCodeNode'; import { upsertHostThemeStyle } from '../utils/hostTheme'; @@ -96,6 +97,7 @@ import { getMeasuredScrollbarGutter } from '../utils/scrollbarGutter'; import { setPreNodeProperties } from '../utils/setWrapperNodeProps'; import { getExpandedRegion, + getHunkAdditionLineRange, getNearestRenderableAdditionLine, getTrailingExpandedRegion, isAdditionLineRenderable, @@ -358,10 +360,12 @@ export class FileDiff< let targetUnifiedIndex: number | undefined; let targetSplitIndex: number | undefined; hunkIterator: for (const hunk of fileDiff.hunks) { - let currentLineNumber = + const hunkStart = side === 'deletions' ? hunk.deletionStart : hunk.additionStart; const hunkCount = side === 'deletions' ? hunk.deletionCount : hunk.additionCount; + let currentLineNumber = + getHunkSideStartBoundary(hunkStart, hunkCount) + 1; let splitIndex = hunk.splitLineStart; let unifiedIndex = hunk.unifiedLineStart; @@ -1059,6 +1063,7 @@ export class FileDiff< this.shouldSelfHealEditSession() ) { finishEditSessionForDiff(this.fileDiff, this.options.parseDiffOptions); + void this.hunksRenderer.refreshHighlightedResult(); } if (expandUnchanged) { this.loadFilesIfNecessary(); @@ -1319,13 +1324,17 @@ export class FileDiff< public attachEditor( editor: DiffsEditor ): (recycle?: boolean) => void { + // Editing is a plain file-diff concern only. Subclasses with their own + // hunk semantics (UnresolvedFile) are not editable, so an editor must + // never attach to them. + if (this.type !== 'file-diff') { + throw new Error( + `FileDiff.attachEditor: cannot attach an editor to a "${this.type}" diff` + ); + } this.editor?.cleanUp(); this.editor = editor; - // Edit sessions are a plain-diff concern; subclasses with their own hunk - // semantics (merge conflicts) keep the per-edit recompute pipeline. - if (this.type === 'file-diff') { - this.hunksRenderer.beginEditSession(this.fileDiffCache); - } + this.hunksRenderer.beginEditSession(); // The editor sync below refuses partial diffs (it needs the full file // contents); kick off hydration so the loaded re-render re-runs it. if (this.fileDiff?.isPartial === true) { @@ -1377,6 +1386,7 @@ export class FileDiff< this.hunksRenderer.setExpandedHunksMap( rebuildExpansionFromAnchors(fileDiff, anchors) ); + void this.hunksRenderer.refreshHighlightedResult(); this.escalateEditSessionRender(); return true; } @@ -1515,7 +1525,8 @@ export class FileDiff< const expandedHunks = this.hunksRenderer.getExpandedHunksMap(); for (const [hunkIndex, hunk] of fileDiff.hunks.entries()) { - if (lineNumber < hunk.additionStart) { + const [hunkStart, hunkEnd] = getHunkAdditionLineRange(hunk); + if (lineNumber < hunkStart) { const region = getExpandedRegion({ isPartial: fileDiff.isPartial, rangeSize: hunk.collapsedBefore, @@ -1523,18 +1534,17 @@ export class FileDiff< hunkIndex, collapsedContextThreshold, }); - const gapStart = hunk.additionStart - region.rangeSize; + const gapStart = hunkStart - region.rangeSize; if ( region.renderAll || lineNumber < gapStart + region.fromStart || - lineNumber >= hunk.additionStart - region.fromEnd + lineNumber >= hunkStart - region.fromEnd ) { return false; } const fromStartDistance = lineNumber - (gapStart + region.fromStart) + 1; - const fromEndDistance = - hunk.additionStart - region.fromEnd - lineNumber; + const fromEndDistance = hunkStart - region.fromEnd - lineNumber; if (fromStartDistance <= fromEndDistance) { this.expandHunk( hunkIndex, @@ -1550,7 +1560,7 @@ export class FileDiff< } return true; } - if (lineNumber < hunk.additionStart + hunk.additionCount) { + if (lineNumber < hunkEnd) { return false; } } @@ -1566,7 +1576,7 @@ export class FileDiff< return false; } const lastHunk = fileDiff.hunks[fileDiff.hunks.length - 1]; - const trailingStart = lastHunk.additionStart + lastHunk.additionCount; + const [, trailingStart] = getHunkAdditionLineRange(lastHunk); if ( lineNumber < trailingStart + trailingRegion.fromStart || lineNumber >= trailingStart + trailingRegion.rangeSize diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index 9787aa0e2..20e3cbd1c 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -7,6 +7,7 @@ import type { DiffsEditableComponent, DiffsEditor, DiffsHighlighter, + EditableInstance, EditorSelection, EditorState, FileContents, @@ -414,6 +415,10 @@ export class Editor implements DiffsEditor { } } + // Small typescript hack to prevent UnresolvedFile from being editable. + edit>( + fileInstance: EditableInstance + ): () => void; edit(fileInstance: DiffsEditableComponent): () => void { if (this.#options.persistState === true && fileInstance.type === 'file') { const file = fileInstance.__getCurrentFile?.(); diff --git a/packages/diffs/src/react/UnresolvedFile.tsx b/packages/diffs/src/react/UnresolvedFile.tsx index ab26cc8db..44c49dce8 100644 --- a/packages/diffs/src/react/UnresolvedFile.tsx +++ b/packages/diffs/src/react/UnresolvedFile.tsx @@ -50,7 +50,7 @@ export interface UnresolvedFileReactOptions export interface UnresolvedFileProps extends Omit< FileDiffProps, - 'fileDiff' | 'options' + 'fileDiff' | 'options' | 'contentEditable' > { file: FileContents; options?: UnresolvedFileReactOptions; diff --git a/packages/diffs/src/renderers/DiffHunksRenderer.ts b/packages/diffs/src/renderers/DiffHunksRenderer.ts index 8f80d66cf..82dd77588 100644 --- a/packages/diffs/src/renderers/DiffHunksRenderer.ts +++ b/packages/diffs/src/renderers/DiffHunksRenderer.ts @@ -53,9 +53,7 @@ import { createPreElement } from '../utils/createPreElement'; import { createSeparator } from '../utils/createSeparator'; import { applySessionChangedLines, - applySessionEditWindow, - findChangedLineWindow, - normalizeEditorLines, + rebuildSessionHunks, remapExpandedHunksForRegionChange, type SessionRegionChange, } from '../utils/editSessionHunks'; @@ -83,7 +81,6 @@ import { iterateOverDiff } from '../utils/iterateOverDiff'; import { renderDiffWithHighlighter } from '../utils/renderDiffWithHighlighter'; import { shouldUseTokenTransformer } from '../utils/shouldUseTokenTransformer'; import { - preserveTrailingEditorBlankLine, recomputeDiffHunksForEdit, recomputeEmptyDocumentDiff, recomputeTopAlignedAdditionDiff, @@ -243,11 +240,6 @@ export class DiffHunksRenderer { // Edit-session state: while active, hunk updates go through the frozen // region skeleton (editSessionHunks) instead of the full recompute. private editSessionActive = false; - // Addition lines as of the last completed hunk-update pass, used by the - // prefix/suffix scan in applyDocumentChange. Line-count passes write dirty - // line text at stale indexes into `diff.additionLines` before the document - // rebuild lands, so the live array can never serve as the "before" side. - private editSessionLines: string[] | undefined; constructor( public options: DiffHunksRendererOptions = { theme: DEFAULT_THEMES }, @@ -284,22 +276,84 @@ export class DiffHunksRenderer { /** * Enter edit-session mode: hunk updates preserve the current region * skeleton instead of recomputing hunks. Called on every editor attach, - * including a re-attach after recycle, which losslessly re-seeds the pass - * snapshot because both hunk-update paths keep `diff.additionLines` - * current. + * including a re-attach after recycle. */ - public beginEditSession(diff: FileDiffMetadata | undefined): void { + public beginEditSession(): void { this.editSessionActive = true; - this.editSessionLines = - diff != null - ? normalizeEditorLines(diff.additionLines).slice() - : undefined; } /** Leave edit-session mode. The exit recompute is the host's concern. */ public endEditSession(): void { this.editSessionActive = false; - this.editSessionLines = undefined; + } + + /** + * Re-highlights the current diff in the background and swaps the fresh + * result in (with a re-render) once it completes. Needed after an edit + * session's exit recompute: session passes plain-fill shifted lines in the + * cached result, and the recompute mutates the diff in place (same object, + * same cacheKey), so identity/cacheKey checks would otherwise treat the + * stale highlight as current forever. The current result — content-correct, + * mostly highlighted — keeps rendering until the fresh one lands, so no + * interim paint drops highlighting. + */ + public refreshHighlightedResult(): Promise { + const { renderCache } = this; + if ( + renderCache == null || + isDiffPlainText(renderCache.diff) || + isDiffMassive(renderCache.diff, this.getTokenizeMaxLength()) + ) { + return Promise.resolve(); + } + const { diff } = renderCache; + const { workerManager } = this; + // The pool's diff cache is keyed by cacheKey, so a worker refresh needs + // one; a keyless diff uses the local highlighter fallback below instead. + if (workerManager?.isWorkingPool() === true && diff.cacheKey != null) { + workerManager.evictDiffFromCache(diff.cacheKey); + return workerManager + .primeDiffHighlightCache(diff) + .then(() => { + this.applyRefreshedResult( + diff, + workerManager.getDiffResultCache(diff) + ); + }) + .catch((error: unknown) => this.onHighlightError(error)); + } + return this.asyncHighlight(diff) + .then((fresh) => this.applyRefreshedResult(diff, fresh)) + .catch((error: unknown) => this.onHighlightError(error)); + } + + // Installs a freshly highlighted result for the same diff, unless the + // renderer moved on while the highlight ran (new diff, options change, or + // a new edit session whose passes the fresh result wouldn't reflect). + private applyRefreshedResult( + diff: FileDiffMetadata, + fresh: RenderDiffResult | undefined + ): void { + if ( + fresh == null || + this.renderCache == null || + this.renderCache.diff !== diff || + this.editSessionActive + ) { + return; + } + const { options } = this.getRenderOptions(diff); + if (!areDiffRenderOptionsEqual(options, fresh.options)) { + return; + } + this.renderCache = { + diff, + options: fresh.options, + highlighted: true, + result: fresh.result, + renderRange: undefined, + }; + this.onRenderUpdate?.(); } public get diffCache(): FileDiffMetadata | undefined { @@ -416,6 +470,7 @@ export class DiffHunksRenderer { const hastLines = result.code.additionLines; const changedAdditionLines: number[] = []; + const previousAdditionLines = new Map(); for (const [line, tokens] of dirtyLines) { const prev = hastLines[line] as HASTElement | undefined; const prevProps = prev?.properties ?? {}; @@ -430,6 +485,7 @@ export class DiffHunksRenderer { diff.additionLines[line] = applyLineTextWithNewline(prevLine, lineText); if (prevText !== lineText) { changedAdditionLines.push(line); + previousAdditionLines.set(line, prevLine); } } hastLines[line] = { @@ -475,16 +531,39 @@ export class DiffHunksRenderer { // genuine changes and are never followed by applyDocumentChange) the // explicit changed indexes are current. if (!lineCountChangeInFlight) { - const changes = applySessionChangedLines( - diff, - changedAdditionLines, - this.options.parseDiffOptions - ); - this.applyExpansionRemaps(changes); - regionsChanged = changes.length > 0; - this.editSessionLines = normalizeEditorLines( - diff.additionLines - ).slice(); + if ( + diff.additionLines.length <= 1 && + diff.additionLines.join('') === '' + ) { + Object.assign( + diff, + recomputeEmptyDocumentDiff(diff, this.options.parseDiffOptions) + ); + this.markEditSessionPass(diff); + regionsChanged = true; + } else if ( + shouldTopAlignAdditionRecompute(diff, diff.additionLines) + ) { + Object.assign( + diff, + recomputeTopAlignedAdditionDiff( + diff, + diff.additionLines, + this.options.parseDiffOptions + ) + ); + this.markEditSessionPass(diff); + regionsChanged = true; + } else { + const change = applySessionChangedLines( + diff, + changedAdditionLines, + this.options.parseDiffOptions, + previousAdditionLines + ); + this.applyExpansionRemap(change); + regionsChanged = change != null; + } } } else { Object.assign( @@ -503,8 +582,8 @@ export class DiffHunksRenderer { return regionsChanged; } - private applyExpansionRemaps(changes: SessionRegionChange[]): void { - for (const change of changes) { + private applyExpansionRemap(change: SessionRegionChange | undefined): void { + if (change != null) { this.expandedHunks = remapExpandedHunksForRegionChange( this.expandedHunks, change @@ -521,57 +600,54 @@ export class DiffHunksRenderer { if (result == null) { return; } + if (diff.isPartial) { + throw new Error('Could not apply document change for partial diff'); + } // updateRenderCache may already have extended diff.additionLines for the // same edit pass, so never bail out purely on matching lengths here. // Read line-by-line from the editor document instead of materializing the // entire text. This preserves blank documents and the final editable empty // row after a trailing line break. + const { additionLines: previousAdditionLines } = diff; diff.additionLines = getEditorDocumentLines( textDocument, - diff.additionLines + previousAdditionLines ); - - const newLength = diff.additionLines.length; - const additionHastLines = result.code.additionLines; - const prevLen = additionHastLines.length; - if (newLength < prevLen) { - additionHastLines.length = newLength; - } - for (let i = prevLen; i < newLength; i++) { - additionHastLines[i] ??= createPlainAdditionLineElement(i, textDocument); - } - if (!diff.isPartial) { - // An empty document splits into zero addition lines, which would recompute - // to a diff with no editable rows and leave the attached host with no - // line element for its caret (the additions column vanishes in split; - // unified shows only deletions). Keep one empty editable line instead. - if ( - diff.additionLines.length <= 1 && - diff.additionLines.join('') === '' - ) { - Object.assign( - diff, - recomputeEmptyDocumentDiff(diff, this.options.parseDiffOptions) - ); - additionHastLines[0] = createPlainAdditionLineElement(0, textDocument); - this.markEditSessionPass(diff); - } else if (this.editSessionActive) { - this.applySessionDocumentChange(diff); - } else { - Object.assign( - diff, - recomputeDiffHunksForEdit(diff, this.options.parseDiffOptions) - ); - } + result.code.additionLines = realignAdditionHastLines( + previousAdditionLines, + diff.additionLines, + result.code.additionLines, + textDocument + ); + // An empty document splits into zero addition lines, which would recompute + // to a diff with no editable rows and leave the attached host with no + // line element for its caret (the additions column vanishes in split; + // unified shows only deletions). Keep one empty editable line instead. + if (diff.additionLines.length <= 1 && diff.additionLines.join('') === '') { + Object.assign( + diff, + recomputeEmptyDocumentDiff(diff, this.options.parseDiffOptions) + ); + result.code.additionLines[0] = createPlainAdditionLineElement( + 0, + textDocument + ); + this.markEditSessionPass(diff); + } else if (this.editSessionActive) { + this.applySessionDocumentChange(diff); + } else { + Object.assign( + diff, + recomputeDiffHunksForEdit(diff, this.options.parseDiffOptions) + ); } this.renderCache.isDirty = true; } - // Session-mode counterpart of the line-count recompute: locate the changed - // window with a prefix/suffix scan of the pass snapshot against the rebuilt - // document lines, then update only the region skeleton it covers. + // Session-mode counterpart of the line-count recompute: derive canonical + // old/current pairing and rebuild the old-side region skeleton from it. private applySessionDocumentChange(diff: FileDiffMetadata): void { const { parseDiffOptions } = this.options; const rawLines = diff.additionLines; @@ -583,36 +659,16 @@ export class DiffHunksRenderer { this.markEditSessionPass(diff); return; } - const previousLines = this.editSessionLines; - const nextLines = normalizeEditorLines(rawLines); - if (previousLines == null) { - // No pass snapshot (the editor attached without diff data): fall back - // to the full recompute for this pass and start tracking from it. - Object.assign(diff, recomputeDiffHunksForEdit(diff, parseDiffOptions)); - this.markEditSessionPass(diff); - return; - } - diff.additionLines = nextLines; - const window = findChangedLineWindow(previousLines, nextLines); - if (window != null) { - const change = applySessionEditWindow(diff, window, parseDiffOptions); - if (change != null) { - this.applyExpansionRemaps([change]); - } - } - preserveTrailingEditorBlankLine(diff, rawLines); - this.editSessionLines = nextLines.slice(); + this.applyExpansionRemap(rebuildSessionHunks(diff, parseDiffOptions)); } - // Records a session pass that replaced hunks wholesale (empty-document / - // top-aligned shims or a snapshotless fallback): the skeleton collapsed to - // the recompute result, and the pass snapshot restarts from it. + // Records a session pass that replaced hunks wholesale (empty-document or + // top-aligned shims). private markEditSessionPass(diff: FileDiffMetadata): void { if (!this.editSessionActive) { return; } diff.editSessionDirty = true; - this.editSessionLines = normalizeEditorLines(diff.additionLines).slice(); } protected getUnifiedLineDecoration({ @@ -813,10 +869,26 @@ export class DiffHunksRenderer { renderRange ); if (this.workerManager?.isWorkingPool() === true) { + // An already-highlighted view is waiting on a fresh highlight for the + // same diff. Returning no result keeps the host's current content in + // place instead of downgrading it to a plain AST; the pending + // highlight's completion will re-render. A different diff or a + // sub-range window still paints plain — the current content cannot + // serve those. + const highlightPending = + this.renderCache.result == null && + this.renderCache.highlighted && + !forcePlainText && + !newContent && + isDefaultRenderRange(renderRange); + if (highlightPending) { + this.renderCache.highlightPending = true; + } if ( - forcePlainText || - this.renderCache.result == null || - (!this.renderCache.highlighted && (newContent || newRenderRange)) + !highlightPending && + (forcePlainText || + this.renderCache.result == null || + (!this.renderCache.highlighted && (newContent || newRenderRange))) ) { this.renderCache.diff = diff; this.renderCache.options = options; @@ -990,6 +1062,7 @@ export class DiffHunksRenderer { } const triggerRenderUpdate = + this.renderCache.highlightPending === true || !this.renderCache.highlighted || !areDiffRenderOptionsEqual(this.renderCache.options, options) || !areDiffTargetsEqual(this.renderCache.diff, diff); @@ -2117,6 +2190,52 @@ function withContentProperties( }; } +// Realigns the cached per-line addition HAST array with an edited document. +// Cached entries are looked up by line index, so a line inserted or removed +// mid-document must shift the surviving entries to their new indexes — +// otherwise rows hidden during the edit (collapsed context) render another +// line's stale tokens once they become visible. Entries outside the changed +// window keep their highlighted content; entries inside it become plain-text +// elements that the editor re-tokenizes on its next background pass. +function realignAdditionHastLines( + previousLines: string[], + nextLines: string[], + hastLines: ElementContent[], + textDocument: DiffsTextDocument +): ElementContent[] { + const maxShared = Math.min(previousLines.length, nextLines.length); + let prefix = 0; + while (prefix < maxShared && previousLines[prefix] === nextLines[prefix]) { + prefix++; + } + let suffix = 0; + while ( + suffix < maxShared - prefix && + previousLines[previousLines.length - 1 - suffix] === + nextLines[nextLines.length - 1 - suffix] + ) { + suffix++; + } + + const realigned: ElementContent[] = new Array(nextLines.length); + for (let index = 0; index < prefix; index++) { + realigned[index] = hastLines[index]; + } + for (let offset = 0; offset < suffix; offset++) { + realigned[nextLines.length - 1 - offset] = + hastLines[previousLines.length - 1 - offset]; + } + // Deferred tokenization can write entries past the previous line count; + // those were produced with post-edit indexes and are already in place. + for (let index = previousLines.length; index < nextLines.length; index++) { + realigned[index] ??= hastLines[index]; + } + for (let index = prefix; index < nextLines.length - suffix; index++) { + realigned[index] ??= createPlainAdditionLineElement(index, textDocument); + } + return realigned; +} + function createPlainAdditionLineElement( lineIndex: number, textDocument: DiffsTextDocument diff --git a/packages/diffs/src/style.css b/packages/diffs/src/style.css index 35f9eb960..170945f08 100644 --- a/packages/diffs/src/style.css +++ b/packages/diffs/src/style.css @@ -813,7 +813,8 @@ [data-diffs-header] ~ [data-diff], [data-diffs-header] ~ [data-file] { [data-code], - &[data-overflow='wrap'] { + &[data-overflow='wrap'], + &[data-dehydrated][data-diff-type='split'][data-overflow='scroll'] { padding-top: 0; } } @@ -842,15 +843,27 @@ background-color: var(--diffs-bg); } - [data-diff-type='split'][data-overflow='wrap'] { + /* Both split layouts that share one grid across the two columns: wrap + * always, and scroll while dehydrated. Scroll mode normally renders each + * column as its own scroll container, where paired annotation rows only get + * equal heights once the resize manager measures them — before hydration + * that measurement hasn't run, so the side hosting the annotation content + * would be taller and the rows below it would shift on hydration. The + * shared grid gives paired rows equal heights in pure CSS; the dehydrated + * scroll variant keeps nowrap text and clips horizontal overflow until + * hydration swaps in the real scroll containers. */ + [data-diff-type='split'][data-overflow='wrap'], + [data-dehydrated][data-diff-type='split'][data-overflow='scroll'] { display: grid; grid-auto-flow: dense; grid-template-columns: repeat(2, var(--diffs-code-grid)); padding-block: var(--diffs-gap-block, var(--diffs-gap-fallback)); - [data-deletions] { + [data-code] { display: contents; + } + [data-deletions] { [data-gutter] { grid-column: 1; } @@ -862,8 +875,6 @@ } [data-additions] { - display: contents; - [data-gutter] { grid-column: 3; border-left: 1px solid var(--diffs-bg); @@ -875,6 +886,11 @@ } } + [data-dehydrated][data-diff-type='split'][data-overflow='scroll'] + [data-content] { + overflow: clip; + } + [data-overflow='scroll'] [data-gutter] { position: sticky; left: 0; diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts index 88d0a4b6e..4af65a42e 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -835,6 +835,9 @@ export interface RenderedDiffASTCache { result: ThemedDiffResult | undefined; renderRange: RenderRange | undefined; isDirty?: boolean; + // A render was skipped while a highlight was in progress; its completion + // will trigger a re-render. + highlightPending?: boolean; } /** @@ -1093,6 +1096,15 @@ export interface DiffsEditableComponent< ) => void; } +// Narrows an editor-attachable instance to exclude UnresolvedFile, which is +// not editable: a `type: 'unresolved-file'` instance maps to `never`, turning +// `editor.edit(new UnresolvedFile())` into a compile error. +export type EditableInstance = T extends { + type: 'unresolved-file'; +} + ? never + : T; + export interface DiffsEditor { /** @internal */ __prepareFile?(file: FileContents): FileContents; @@ -1107,7 +1119,9 @@ export interface DiffsEditor { | undefined, renderRange: RenderRange | undefined ): void; - edit(fileInstance: DiffsEditableComponent): () => void; + edit>( + fileInstance: EditableInstance + ): () => void; cleanUp(recycle?: boolean): void; } diff --git a/packages/diffs/src/utils/editSessionHunks.ts b/packages/diffs/src/utils/editSessionHunks.ts index ce483047e..12070712b 100644 --- a/packages/diffs/src/utils/editSessionHunks.ts +++ b/packages/diffs/src/utils/editSessionHunks.ts @@ -7,10 +7,11 @@ import type { Hunk, HunkExpansionRegion, } from '../types'; +import { getHunkSideStartBoundary } from './getHunkSideBoundaries'; import { parseDiffFromFile } from './parseDiffFromFile'; -import { slideBlankBoundaryBlocksUp } from './realignChangeContent'; import { offsetHunkContent, + preserveTrailingEditorBlankLine, recomputeDiffHunksForEdit, recomputeDiffRenderLineCounts, recomputeHunkRenderLineCounts, @@ -21,34 +22,26 @@ import { getTrailingExpandedRegion, } from './virtualDiffLayout'; -// While an editor is attached to a FileDiff, hunks are treated as a frozen -// "region skeleton": each hunk is one region spanning its full range, regions -// never merge/split/drop on their own, and each edit re-diffs only the region -// it lands in. A region whose re-diff comes back empty persists as a -// context-only hunk so its rows keep rendering. The functions here implement -// that per-edit region math; the real hunk recompute runs once on genuine -// session exit (finishEditSessionForDiff). +// While an editor is attached to a FileDiff, each hunk is a persistent region +// identified by its old-side range. Structural passes rebuild those regions +// from one canonical old/current diff; a reverted region remains as context so +// its rows keep rendering until the genuine session-exit recompute. -/** - * The addition-line span an edit changed, from a common prefix/suffix scan. - * `start` is the first changed line (identical in pre/post-edit coordinates), - * `prevEnd`/`nextEnd` are the exclusive ends in pre/post-edit lines. - */ -export interface ChangedLineWindow { +export interface DivergenceCore { start: number; - prevEnd: number; - nextEnd: number; + deletionEnd: number; + additionEnd: number; } -/** A structural change to the region skeleton (rendered row set changed). */ -export type SessionRegionChange = - | { - type: 'merge'; - firstIndex: number; - lastIndex: number; - previousHunkCount: number; - } - | { type: 'insert'; index: number; previousHunkCount: number }; +interface PreviousRegionSpan { + firstIndex: number; + lastIndex: number; +} + +/** Maps rebuilt regions back to the previous skeleton for expansion remapping. */ +export interface SessionRegionChange { + regions: Array; +} interface RegionBounds { additionStart: number; @@ -57,6 +50,18 @@ interface RegionBounds { deletionEnd: number; } +interface RegionPlan { + deletionStart: number; + deletionEnd: number; + blocks: ChangeContent[]; + previousSpan: PreviousRegionSpan | undefined; +} + +const deletionLineSetCache = new WeakMap< + FileDiffMetadata, + { lines: string[]; set: Set } +>(); + /** * Drops the editor document's phantom trailing empty line (a document ending * in a newline exposes one extra empty line the parsed diff never contains) @@ -70,243 +75,223 @@ export function normalizeEditorLines(lines: string[]): string[] { } /** - * Common prefix/suffix scan between the last completed pass's lines and the - * rebuilt document lines. Returns undefined when nothing changed. + * Find the complete old/current divergence core. The old side is immutable + * during an edit session, so this result needs no prior-pass snapshot. */ -export function findChangedLineWindow( - previousLines: string[], - nextLines: string[] -): ChangedLineWindow | undefined { - const maxStart = Math.min(previousLines.length, nextLines.length); +export function findDivergenceCore( + deletionLines: string[], + additionLines: string[] +): DivergenceCore | undefined { + const maxStart = Math.min(deletionLines.length, additionLines.length); let start = 0; - while (start < maxStart && previousLines[start] === nextLines[start]) { + while (start < maxStart && deletionLines[start] === additionLines[start]) { start++; } - let prevEnd = previousLines.length; - let nextEnd = nextLines.length; + let deletionEnd = deletionLines.length; + let additionEnd = additionLines.length; while ( - prevEnd > start && - nextEnd > start && - previousLines[prevEnd - 1] === nextLines[nextEnd - 1] + deletionEnd > start && + additionEnd > start && + deletionLines[deletionEnd - 1] === additionLines[additionEnd - 1] ) { - prevEnd--; - nextEnd--; + deletionEnd--; + additionEnd--; } - if (start === prevEnd && start === nextEnd) { + if (start === deletionEnd && start === additionEnd) { return undefined; } - return { start, prevEnd, nextEnd }; + return { start, deletionEnd, additionEnd }; } /** - * Applies one contiguous changed window to the session skeleton: regions the - * window touches merge/grow to cover it (absorbing unchanged gap lines in - * paired old/new correspondence), a window wholly inside a gap synthesizes a - * new region, and regions after the window shift by the line delta. The - * covered region is re-diffed in place. Returns the structural change when - * the region set itself changed shape (merge/growth/synthesis). + * Rebuild the session skeleton as a pure function of the immutable old lines, + * current new lines, and old-side ranges of the previous regions. One + * canonical parse supplies the same change blocks that session exit will use. */ -export function applySessionEditWindow( +export function rebuildSessionHunks( diff: FileDiffMetadata, - window: ChangedLineWindow, parseDiffOptions?: CreatePatchOptionsNonabortable ): SessionRegionChange | undefined { - const { hunks } = diff; - const delta = window.nextEnd - window.prevEnd; - diff.editSessionDirty = true; - - if (hunks.length === 0) { - const bounds: RegionBounds = { - additionStart: 0, - additionEnd: diff.additionLines.length, - deletionStart: 0, - deletionEnd: diff.deletionLines.length, - }; - hunks.push(rediffRegion(diff, bounds, parseDiffOptions)); - finalizeSessionHunks(diff, 0); - return { type: 'insert', index: 0, previousHunkCount: 0 }; - } - - // Locate the span of regions the window touches, treating windows adjacent - // to a region edge as touching so boundary edits grow the region instead of - // synthesizing a sibling next to it. - let firstIndex = -1; - let lastIndex = -1; - for (let index = 0; index < hunks.length; index++) { - const hunk = hunks[index]; - const regionStart = hunk.additionLineIndex; - const regionEnd = regionStart + hunk.additionCount; - if (regionStart > window.prevEnd) { - break; - } - if (window.start <= regionEnd) { - if (firstIndex === -1) { - firstIndex = index; - } - lastIndex = index; - } - } - - if (firstIndex === -1) { - return synthesizeRegion(diff, window, delta, parseDiffOptions); - } - - const firstHunk = hunks[firstIndex]; - const lastHunk = hunks[lastIndex]; - const lastHunkEnd = lastHunk.additionLineIndex + lastHunk.additionCount; - const additionStart = Math.min(firstHunk.additionLineIndex, window.start); - const additionEndBefore = Math.max(lastHunkEnd, window.prevEnd); - // Gap lines are 1:1 paired with old-side lines, so growing a region into a - // gap absorbs the same number of deletion lines from that gap. - const bounds: RegionBounds = { - additionStart, - additionEnd: additionEndBefore + delta, - deletionStart: - firstHunk.deletionLineIndex - - (firstHunk.additionLineIndex - additionStart), - deletionEnd: - lastHunk.deletionLineIndex + - lastHunk.deletionCount + - (additionEndBefore - lastHunkEnd), - }; - const regionHunk = rediffRegion(diff, bounds, parseDiffOptions); - if (delta !== 0) { - for (let index = lastIndex + 1; index < hunks.length; index++) { - shiftHunkAdditionCoords(hunks[index], delta); - } - } - const previousHunkCount = hunks.length; - hunks.splice(firstIndex, lastIndex - firstIndex + 1, regionHunk); - finalizeSessionHunks(diff, firstIndex); + const previousHunks = diff.hunks; + const editorAdditionLines = diff.additionLines; + const canonicalAdditionLines = normalizeEditorLines(editorAdditionLines); + const canonicalDiff = + canonicalAdditionLines === editorAdditionLines + ? diff + : { ...diff, additionLines: canonicalAdditionLines }; + const blocks = parseCanonicalChangeBlocks(canonicalDiff, parseDiffOptions); + const plans = buildRegionPlans( + previousHunks, + blocks, + diff.deletionLines.length + ); - const regionGrew = - additionStart < firstHunk.additionLineIndex || - additionEndBefore > lastHunkEnd; - if (firstIndex === lastIndex && !regionGrew) { + const nextHunks = buildRegionHunks(canonicalDiff, plans); + diff.additionLines = canonicalAdditionLines; + diff.hunks = nextHunks; + diff.editSessionDirty = true; + finalizeSessionHunks(diff); + preserveTrailingEditorBlankLine(diff, editorAdditionLines); + const layoutChanged = hasRegionOrSplitLayoutChanged( + previousHunks, + diff.hunks + ); + if (!layoutChanged) { return undefined; } - return { type: 'merge', firstIndex, lastIndex, previousHunkCount }; + return { regions: plans.map((plan) => plan.previousSpan) }; } /** - * Applies a set of changed addition-line indexes from a pass with no line - * count change: lines inside a region re-diff that region in place; lines in - * a gap synthesize a region per gap (one window spanning that gap's changed - * lines). Returns the structural changes, if any, for escalation/remapping. + * Keep a cheap content-only path when a same-line-count pass cannot alter the + * canonical blocks. Gap, ambiguous, or multi-region edits rebuild statelessly. */ export function applySessionChangedLines( diff: FileDiffMetadata, changedAdditionLineIndexes: Iterable, - parseDiffOptions?: CreatePatchOptionsNonabortable -): SessionRegionChange[] { + parseDiffOptions?: CreatePatchOptionsNonabortable, + previousAdditionLines?: ReadonlyMap +): SessionRegionChange | undefined { const lines = Array.from(new Set(changedAdditionLineIndexes)) .filter((line) => line >= 0 && line < diff.additionLines.length) .sort((a, b) => a - b); if (lines.length === 0) { - return []; + return undefined; } const { hunks } = diff; - const regionIndexes = new Set(); - // Changed lines outside every region, grouped per gap (keyed by the index - // of the region that follows the gap) into one [start, end) window each. - const gapWindows: Array<[start: number, end: number]> = []; + let regionIndex: number | undefined; let hunkIndex = 0; - let currentGapKey = -1; for (const line of lines) { - while ( - hunkIndex < hunks.length && - line >= - hunks[hunkIndex].additionLineIndex + hunks[hunkIndex].additionCount - ) { + while (hunkIndex < hunks.length) { + const hunk = hunks[hunkIndex]; + const start = getHunkAdditionStart(hunk); + if (line < start + hunk.additionCount) break; hunkIndex++; } const hunk = hunks[hunkIndex]; - if (hunk != null && line >= hunk.additionLineIndex) { - regionIndexes.add(hunkIndex); - continue; - } - const lastWindow = gapWindows[gapWindows.length - 1]; - if (currentGapKey === hunkIndex && lastWindow != null) { - lastWindow[1] = line + 1; - } else { - currentGapKey = hunkIndex; - gapWindows.push([line, line + 1]); + const start = hunk == null ? undefined : getHunkAdditionStart(hunk); + if ( + start == null || + line < start || + (regionIndex != null && regionIndex !== hunkIndex) + ) { + return rebuildSessionHunks(diff, parseDiffOptions); } + regionIndex = hunkIndex; } - // In-place region re-diffs first, while region indexes are still valid. - for (const index of regionIndexes) { - const hunk = hunks[index]; - const bounds: RegionBounds = { - additionStart: hunk.additionLineIndex, - additionEnd: hunk.additionLineIndex + hunk.additionCount, - deletionStart: hunk.deletionLineIndex, - deletionEnd: hunk.deletionLineIndex + hunk.deletionCount, - }; - hunks[index] = rediffRegion(diff, bounds, parseDiffOptions); - syncHunkNoEOFCRFromFullFile(diff, index); - diff.editSessionDirty = true; - } - if (regionIndexes.size > 0) { - recomputeDiffRenderLineCounts(diff); + if (regionIndex == null) { + return undefined; } - - // Gap windows in descending order so earlier windows keep valid coordinates - // while later synthesis inserts hunks behind them. - const changes: SessionRegionChange[] = []; - for (let index = gapWindows.length - 1; index >= 0; index--) { - const [start, end] = gapWindows[index]; - const change = applySessionEditWindow( + if ( + canRetainCanonicalBlocks( diff, - { start, prevEnd: end, nextEnd: end }, + lines, + regionIndex, + previousAdditionLines, parseDiffOptions - ); - if (change != null) { - changes.push(change); + ) + ) { + diff.editSessionDirty = true; + return undefined; + } + return rebuildSessionHunks(diff, parseDiffOptions); +} + +// Existing balanced change blocks remain canonical when every edited line was +// unmatched before and stays unmatched now: the old/new equality matrix is +// unchanged, and similarity realignment never reorders a balanced block. +function canRetainCanonicalBlocks( + diff: FileDiffMetadata, + changedLines: number[], + regionIndex: number, + previousAdditionLines: ReadonlyMap | undefined, + parseDiffOptions: CreatePatchOptionsNonabortable | undefined +): boolean { + if ( + previousAdditionLines == null || + parseDiffOptions?.ignoreWhitespace === true || + parseDiffOptions?.stripTrailingCr === true + ) { + return false; + } + const deletionLineSet = getDeletionLineSet(diff); + const hunk = diff.hunks[regionIndex]; + if (hunk.hunkContent.some(isPureChange)) { + return false; + } + for (const line of changedLines) { + const previousLine = previousAdditionLines.get(line); + const additionLine = diff.additionLines[line]; + if ( + previousLine == null || + additionLine == null || + deletionLineSet.has(previousLine) || + deletionLineSet.has(additionLine) + ) { + return false; + } + let insideBalancedChange = false; + for (const content of hunk.hunkContent) { + if ( + content.type === 'change' && + content.additions === content.deletions && + line >= content.additionLineIndex && + line < content.additionLineIndex + content.additions + ) { + insideBalancedChange = true; + break; + } + } + if (!insideBalancedChange) { + return false; } } - return changes; + return true; } -/** - * Rebuilds hunk-gap expansion keys after a structural region change so - * expansion state stays anchored to the same gaps. Merged-away gap keys drop; - * a synthesized region splits its gap's expansion across the two new gaps. - * The trailing pseudo-key (old hunk count) follows the same shift. - */ +function getDeletionLineSet(diff: FileDiffMetadata): Set { + const cached = deletionLineSetCache.get(diff); + if (cached?.lines === diff.deletionLines) { + return cached.set; + } + const set = new Set(diff.deletionLines); + deletionLineSetCache.set(diff, { lines: diff.deletionLines, set }); + return set; +} + +function isPureChange( + content: ContextContent | ChangeContent | undefined +): boolean { + return ( + content?.type === 'change' && + (content.additions === 0 || content.deletions === 0) + ); +} + +/** Preserve expansion at the surviving outer edges of rebuilt old-side gaps. */ export function remapExpandedHunksForRegionChange( expandedHunks: Map, change: SessionRegionChange ): Map { const remapped = new Map(); - if (change.type === 'merge') { - const removed = change.lastIndex - change.firstIndex; - for (const [key, region] of expandedHunks) { - if (key <= change.firstIndex) { - remapped.set(key, region); - } else if (key > change.lastIndex) { - remapped.set(key - removed, region); - } - // Keys inside (firstIndex, lastIndex] were gaps the merge absorbed. - } - return remapped; - } - for (const [key, region] of expandedHunks) { - if (key < change.index) { - remapped.set(key, region); - } else if (key > change.index) { - remapped.set(key + 1, region); - } else { - // The gap this key described was split by the new region: keep its - // start-anchored expansion on the first half and its end-anchored - // expansion on the second. - if (region.fromStart > 0) { - remapped.set(key, { fromStart: region.fromStart, fromEnd: 0 }); - } - if (region.fromEnd > 0) { - remapped.set(key + 1, { fromStart: 0, fromEnd: region.fromEnd }); - } + const { regions } = change; + for (let key = 0; key <= regions.length; key++) { + const previous = regions[key - 1]; + const next = regions[key]; + const fromStartSource = + key === 0 + ? expandedHunks.get(0) + : previous == null + ? undefined + : expandedHunks.get(previous.lastIndex + 1); + const fromEndSource = + next == null ? undefined : expandedHunks.get(next.firstIndex); + const fromStart = fromStartSource?.fromStart ?? 0; + const fromEnd = fromEndSource?.fromEnd ?? 0; + if (fromStart > 0 || fromEnd > 0) { + remapped.set(key, { fromStart, fromEnd }); } } return remapped; @@ -343,7 +328,7 @@ export function captureExpansionAnchors( if (region.rangeSize <= collapsedContextThreshold) { continue; } - const gapEnd = hunk.deletionLineIndex; + const gapEnd = getHunkDeletionStart(hunk); const gapStart = gapEnd - region.rangeSize; if (region.fromStart > 0) { anchors.push([gapStart, gapStart + region.fromStart]); @@ -365,7 +350,7 @@ export function captureExpansionAnchors( trailingRegion.rangeSize > collapsedContextThreshold ) { const lastHunk = diff.hunks[diff.hunks.length - 1]; - const gapStart = lastHunk.deletionLineIndex + lastHunk.deletionCount; + const gapStart = getHunkDeletionStart(lastHunk) + lastHunk.deletionCount; anchors.push([gapStart, gapStart + trailingRegion.fromStart]); } return anchors; @@ -407,17 +392,14 @@ export function rebuildExpansionFromAnchors( } }; for (const [hunkIndex, hunk] of diff.hunks.entries()) { - applyGap( - hunkIndex, - hunk.deletionLineIndex - Math.max(hunk.collapsedBefore, 0), - hunk.deletionLineIndex - ); + const gapEnd = getHunkDeletionStart(hunk); + applyGap(hunkIndex, gapEnd - Math.max(hunk.collapsedBefore, 0), gapEnd); } const lastHunk = diff.hunks[diff.hunks.length - 1]; if (lastHunk != null && !diff.isPartial && diff.deletionLines.length > 0) { applyGap( diff.hunks.length, - lastHunk.deletionLineIndex + lastHunk.deletionCount, + getHunkDeletionStart(lastHunk) + lastHunk.deletionCount, diff.deletionLines.length ); } @@ -441,181 +423,262 @@ export function finishEditSessionForDiff( return true; } -// Synthesizes a region for a window that touches no existing region. When the -// window is empty on either side (pure insert/delete inside a gap), one -// adjacent context line is absorbed so the re-diff anchors on real content. -function synthesizeRegion( +// Parse the complete old/current files once with the same context policy as +// session exit. Equal boundary lines affect Myers tie-breaking, while parsed +// context is required for the blank-run alignment post-pass; only canonical +// change blocks are retained. Running cursors normalize unified N,0 indexes. +function parseCanonicalChangeBlocks( diff: FileDiffMetadata, - window: ChangedLineWindow, - delta: number, parseDiffOptions?: CreatePatchOptionsNonabortable -): SessionRegionChange { - const { hunks } = diff; - let insertIndex = 0; - while ( - insertIndex < hunks.length && - hunks[insertIndex].additionLineIndex < window.start - ) { - insertIndex++; - } - - // Constant pairing offset between old and new line indexes within this gap. - const pairOffset = - insertIndex < hunks.length - ? hunks[insertIndex].deletionLineIndex - - hunks[insertIndex].additionLineIndex - : hunks[insertIndex - 1].deletionLineIndex + - hunks[insertIndex - 1].deletionCount - - (hunks[insertIndex - 1].additionLineIndex + - hunks[insertIndex - 1].additionCount); - - let { start, prevEnd, nextEnd } = window; - if (start === prevEnd || start === nextEnd) { - if (start > 0) { - start--; - } else if (nextEnd < diff.additionLines.length) { - prevEnd++; - nextEnd++; - } +): ChangeContent[] { + if (findDivergenceCore(diff.deletionLines, diff.additionLines) == null) { + return []; } + const parsed = parseDiffFromFile( + { + name: diff.prevName ?? diff.name, + contents: diff.deletionLines.join(''), + }, + { + name: diff.name, + contents: diff.additionLines.join(''), + lang: diff.lang, + }, + parseDiffOptions + ); - const bounds: RegionBounds = { - additionStart: start, - additionEnd: nextEnd, - deletionStart: start + pairOffset, - deletionEnd: prevEnd + pairOffset, - }; - const regionHunk = rediffRegion(diff, bounds, parseDiffOptions); - if (delta !== 0) { - for (let index = insertIndex; index < hunks.length; index++) { - shiftHunkAdditionCoords(hunks[index], delta); + const blocks: ChangeContent[] = []; + let coveredAdditions = 0; + let coveredDeletions = 0; + for (const hunk of parsed.hunks) { + const contextLines = + hunk.additionCount > 0 + ? hunk.additionLineIndex - coveredAdditions + : hunk.deletionLineIndex - coveredDeletions; + coveredAdditions += contextLines; + coveredDeletions += contextLines; + for (const content of hunk.hunkContent) { + if (content.type === 'context') { + coveredAdditions += content.lines; + coveredDeletions += content.lines; + continue; + } + const block = offsetHunkContent(content, 0, 0) as ChangeContent; + if (block.additions === 0) { + block.additionLineIndex = coveredAdditions; + } + if (block.deletions === 0) { + block.deletionLineIndex = coveredDeletions; + } + blocks.push(block); + coveredAdditions += block.additions; + coveredDeletions += block.deletions; } } - const previousHunkCount = hunks.length; - hunks.splice(insertIndex, 0, regionHunk); - finalizeSessionHunks(diff, insertIndex); - return { type: 'insert', index: insertIndex, previousHunkCount }; + return blocks; } -// Re-diffs a region's old/new slices with zero context, producing one Hunk -// spanning the region's full range. Zero result hunks yield a context-only -// hunk (region persists while the session is active); several result hunks -// merge into one hunkContent with the identical stretches between them -// re-expressed as context blocks. -function rediffRegion( - diff: FileDiffMetadata, - bounds: RegionBounds, - parseDiffOptions?: CreatePatchOptionsNonabortable -): Hunk { - const deletionSlice = diff.deletionLines.slice( - bounds.deletionStart, - bounds.deletionEnd - ); - const additionSlice = diff.additionLines.slice( - bounds.additionStart, - bounds.additionEnd - ); - const additionCount = additionSlice.length; - const deletionCount = deletionSlice.length; - - const hunkContent: (ContextContent | ChangeContent)[] = []; - let additionChangedLines = 0; - let deletionChangedLines = 0; - - const pushContext = ( - lines: number, - additionLineIndex: number, - deletionLineIndex: number - ) => { - if (lines > 0) { - hunkContent.push({ - type: 'context', - lines, - additionLineIndex, - deletionLineIndex, - }); +// Co-walk canonical blocks and previous old-side regions. A block touching a +// region grows it; touching several merges them; a block wholly in a gap gets +// its own region. Pure insertions/deletions absorb one adjacent context line +// when available so the region stays renderable on both sides and after undo. +function buildRegionPlans( + previousHunks: Hunk[], + blocks: ChangeContent[], + deletionLineCount: number +): RegionPlan[] { + const previousPlans = previousHunks.map((hunk, index): RegionPlan => { + const deletionStart = getHunkDeletionStart(hunk); + return { + deletionStart, + deletionEnd: deletionStart + hunk.deletionCount, + blocks: [], + previousSpan: { firstIndex: index, lastIndex: index }, + }; + }); + const plans: RegionPlan[] = []; + let previousIndex = 0; + + for (const block of blocks) { + const blockStart = block.deletionLineIndex; + const blockEnd = blockStart + block.deletions; + while ( + previousIndex < previousPlans.length && + previousPlans[previousIndex].deletionEnd < blockStart + ) { + plans.push(previousPlans[previousIndex]); + previousIndex++; } - }; - if (deletionSlice.join('') === additionSlice.join('')) { - pushContext(additionCount, bounds.additionStart, bounds.deletionStart); - } else { - const reparsed = parseDiffFromFile( - { - name: diff.prevName ?? diff.name, - contents: deletionSlice.join(''), - }, - { - name: diff.name, - contents: additionSlice.join(''), - lang: diff.lang, - }, - { ...parseDiffOptions, context: 0 } - ); - // Walk the zero-context hunks, re-deriving the unchanged stretches - // between them (context is paired, so both sides advance by the same - // amount). The stretch length must come from a side the hunk has content - // on: a zero-count side's start follows the unified `N,0` convention - // (the line *before* the change), so its lineIndex is one short of the - // lines consumed ahead of the hunk. - let coveredAdditions = 0; - let coveredDeletions = 0; - for (const parsedHunk of reparsed.hunks) { - const contextLines = - parsedHunk.additionCount > 0 - ? parsedHunk.additionLineIndex - coveredAdditions - : parsedHunk.deletionLineIndex - coveredDeletions; - pushContext( - contextLines, - bounds.additionStart + coveredAdditions, - bounds.deletionStart + coveredDeletions - ); - coveredAdditions += contextLines; - coveredDeletions += contextLines; - for (const content of parsedHunk.hunkContent) { - // Parsed content indexes are relative to the slice; offset them into - // full-file coordinates. A zero-count side carries the `N,0` - // convention (one short of the lines consumed), so pin it to the - // running counter instead. - const offset = offsetHunkContent( - content, - bounds.additionStart, - bounds.deletionStart - ); - if (offset.type === 'change') { - if (offset.additions === 0) { - offset.additionLineIndex = bounds.additionStart + coveredAdditions; - } - if (offset.deletions === 0) { - offset.deletionLineIndex = bounds.deletionStart + coveredDeletions; - } + let plan = + plans.length > 0 && + blockTouchesRegion(blockStart, blockEnd, plans[plans.length - 1]) + ? plans.pop() + : undefined; + while ( + previousIndex < previousPlans.length && + previousPlans[previousIndex].deletionStart <= blockEnd + ) { + plan = mergeRegionPlans(plan, previousPlans[previousIndex]); + previousIndex++; + } + + if (plan == null) { + let deletionStart = blockStart; + let deletionEnd = blockEnd; + if ( + (block.deletions === 0 || block.additions === 0) && + deletionLineCount > block.deletions + ) { + const previousEnd = plans[plans.length - 1]?.deletionEnd ?? 0; + const nextStart = + previousPlans[previousIndex]?.deletionStart ?? deletionLineCount; + if (blockStart > previousEnd) { + deletionStart--; + } else if (blockEnd < nextStart) { + deletionEnd++; } - hunkContent.push(offset); } - additionChangedLines += parsedHunk.additionLines; - deletionChangedLines += parsedHunk.deletionLines; - coveredAdditions += parsedHunk.additionCount; - coveredDeletions += parsedHunk.deletionCount; + plan = { + deletionStart, + deletionEnd, + blocks: [], + previousSpan: undefined, + }; } - pushContext( - additionCount - coveredAdditions, - bounds.additionStart + coveredAdditions, - bounds.deletionStart + coveredDeletions + plan.deletionStart = Math.min(plan.deletionStart, blockStart); + plan.deletionEnd = Math.max(plan.deletionEnd, blockEnd); + plan.blocks.push(block); + plans.push(plan); + } + + while (previousIndex < previousPlans.length) { + plans.push(previousPlans[previousIndex]); + previousIndex++; + } + return plans; +} + +function blockTouchesRegion( + blockStart: number, + blockEnd: number, + region: RegionPlan +): boolean { + return blockStart <= region.deletionEnd && blockEnd >= region.deletionStart; +} + +function mergeRegionPlans( + target: RegionPlan | undefined, + source: RegionPlan +): RegionPlan { + if (target == null) { + return source; + } + target.deletionStart = Math.min(target.deletionStart, source.deletionStart); + target.deletionEnd = Math.max(target.deletionEnd, source.deletionEnd); + target.blocks.push(...source.blocks); + if (source.previousSpan != null) { + target.previousSpan ??= { ...source.previousSpan }; + target.previousSpan.firstIndex = Math.min( + target.previousSpan.firstIndex, + source.previousSpan.firstIndex + ); + target.previousSpan.lastIndex = Math.max( + target.previousSpan.lastIndex, + source.previousSpan.lastIndex ); } + return target; +} + +// One paired-context walk constructs every region's new-side range. There is +// no downstream coordinate shifting: each boundary is derived from the +// canonical blocks that precede it. +function buildRegionHunks(diff: FileDiffMetadata, plans: RegionPlan[]): Hunk[] { + const hunks: Hunk[] = []; + let deletionCursor = 0; + let additionCursor = 0; + for (const plan of plans) { + const contextBefore = plan.deletionStart - deletionCursor; + if (contextBefore < 0) { + throw new Error('buildRegionHunks: overlapping old-side regions'); + } + deletionCursor += contextBefore; + additionCursor += contextBefore; + const additionStart = additionCursor; + const hunkContent: Array = []; + + for (const canonicalBlock of plan.blocks) { + const deletionContext = canonicalBlock.deletionLineIndex - deletionCursor; + const additionContext = canonicalBlock.additionLineIndex - additionCursor; + if (deletionContext < 0 || deletionContext !== additionContext) { + throw new Error('buildRegionHunks: canonical block context mismatch'); + } + pushContext(hunkContent, deletionContext, additionCursor, deletionCursor); + deletionCursor += deletionContext; + additionCursor += additionContext; + hunkContent.push({ ...canonicalBlock }); + deletionCursor += canonicalBlock.deletions; + additionCursor += canonicalBlock.additions; + } + const trailingContext = plan.deletionEnd - deletionCursor; + if (trailingContext < 0) { + throw new Error('buildRegionHunks: block exceeds its old-side region'); + } + pushContext(hunkContent, trailingContext, additionCursor, deletionCursor); + deletionCursor += trailingContext; + additionCursor += trailingContext; + hunks.push( + createRegionHunk( + diff, + { + additionStart, + additionEnd: additionCursor, + deletionStart: plan.deletionStart, + deletionEnd: plan.deletionEnd, + }, + hunkContent + ) + ); + } + + if ( + diff.deletionLines.length - deletionCursor !== + diff.additionLines.length - additionCursor + ) { + throw new Error('buildRegionHunks: trailing context mismatch'); + } + return hunks; +} + +function createRegionHunk( + diff: FileDiffMetadata, + bounds: RegionBounds, + hunkContent: Array +): Hunk { + const additionCount = bounds.additionEnd - bounds.additionStart; + const deletionCount = bounds.deletionEnd - bounds.deletionStart; + let additionLines = 0; + let deletionLines = 0; + for (const content of hunkContent) { + if (content.type === 'change') { + additionLines += content.additions; + deletionLines += content.deletions; + } + } const hunk: Hunk = { collapsedBefore: 0, - additionStart: bounds.additionStart + 1, + additionStart: getUnifiedStart(bounds.additionStart, additionCount), additionCount, - additionLines: additionChangedLines, - additionLineIndex: bounds.additionStart, - deletionStart: bounds.deletionStart + 1, + additionLines, + additionLineIndex: getUnifiedLineIndex(bounds.additionStart, additionCount), + deletionStart: getUnifiedStart(bounds.deletionStart, deletionCount), deletionCount, - deletionLines: deletionChangedLines, - deletionLineIndex: bounds.deletionStart, + deletionLines, + deletionLineIndex: getUnifiedLineIndex(bounds.deletionStart, deletionCount), hunkContent, - hunkSpecs: `@@ -${bounds.deletionStart + 1},${deletionCount} +${bounds.additionStart + 1},${additionCount} @@`, + hunkSpecs: `@@ -${getUnifiedStart(bounds.deletionStart, deletionCount)},${deletionCount} +${getUnifiedStart(bounds.additionStart, additionCount)},${additionCount} @@`, splitLineStart: 0, splitLineCount: 0, unifiedLineStart: 0, @@ -623,23 +686,116 @@ function rediffRegion( noEOFCRAdditions: false, noEOFCRDeletions: false, }; - // The inner parse runs with zero context, so blank-run slides can only - // apply once the context blocks are reassembled here — keeping the session - // rendering identical to the exit parse, which slides in its own post-pass. - slideBlankBoundaryBlocksUp(hunk, diff); recomputeHunkRenderLineCounts(hunk); return hunk; } -function shiftHunkAdditionCoords(hunk: Hunk, delta: number): void { - hunk.additionLineIndex += delta; - hunk.additionStart += delta; +function pushContext( + hunkContent: Array, + lines: number, + additionLineIndex: number, + deletionLineIndex: number +): void { + if (lines > 0) { + hunkContent.push({ + type: 'context', + lines, + additionLineIndex, + deletionLineIndex, + }); + } +} + +function hasRegionOrSplitLayoutChanged( + previous: Hunk[], + next: Hunk[] +): boolean { + if (previous.length !== next.length) { + return true; + } + for (let index = 0; index < previous.length; index++) { + const previousHunk = previous[index]; + const nextHunk = next[index]; + if ( + getHunkDeletionStart(previousHunk) !== getHunkDeletionStart(nextHunk) || + previousHunk.deletionCount !== nextHunk.deletionCount || + getHunkAdditionStart(previousHunk) !== getHunkAdditionStart(nextHunk) || + previousHunk.additionCount !== nextHunk.additionCount || + previousHunk.splitLineCount !== nextHunk.splitLineCount || + !haveSameSplitRowMapping(previousHunk, nextHunk) + ) { + return true; + } + } + return false; +} + +function haveSameSplitRowMapping(previous: Hunk, next: Hunk): boolean { + const previousRows = iterateSplitRowMapping(previous); + const nextRows = iterateSplitRowMapping(next); + while (true) { + const previousRow = previousRows.next(); + const nextRow = nextRows.next(); + if (previousRow.done === true || nextRow.done === true) { + return previousRow.done === nextRow.done; + } + if ( + previousRow.value[0] !== nextRow.value[0] || + previousRow.value[1] !== nextRow.value[1] + ) { + return false; + } + } +} + +function* iterateSplitRowMapping( + hunk: Hunk +): Generator< + [deletionLine: number | undefined, additionLine: number | undefined] +> { for (const content of hunk.hunkContent) { - content.additionLineIndex += delta; + if (content.type === 'context') { + for (let offset = 0; offset < content.lines; offset++) { + yield [ + content.deletionLineIndex + offset, + content.additionLineIndex + offset, + ]; + } + continue; + } + const rowCount = Math.max(content.deletions, content.additions); + for (let offset = 0; offset < rowCount; offset++) { + yield [ + offset < content.deletions + ? content.deletionLineIndex + offset + : undefined, + offset < content.additions + ? content.additionLineIndex + offset + : undefined, + ]; + } } } -function finalizeSessionHunks(diff: FileDiffMetadata, hunkIndex: number): void { +function getHunkAdditionStart(hunk: Hunk): number { + return getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount); +} + +function getHunkDeletionStart(hunk: Hunk): number { + return getHunkSideStartBoundary(hunk.deletionStart, hunk.deletionCount); +} + +function getUnifiedStart(lineIndex: number, count: number): number { + return count === 0 ? lineIndex : lineIndex + 1; +} + +function getUnifiedLineIndex(lineIndex: number, count: number): number { + return count === 0 ? lineIndex - 1 : lineIndex; +} + +function finalizeSessionHunks(diff: FileDiffMetadata): void { recomputeDiffRenderLineCounts(diff); - syncHunkNoEOFCRFromFullFile(diff, hunkIndex); + for (let index = 0; index < diff.hunks.length; index++) { + syncHunkNoEOFCRFromFullFile(diff, index); + } } diff --git a/packages/diffs/src/utils/getHunkSideBoundaries.ts b/packages/diffs/src/utils/getHunkSideBoundaries.ts new file mode 100644 index 000000000..2d34b786d --- /dev/null +++ b/packages/diffs/src/utils/getHunkSideBoundaries.ts @@ -0,0 +1,8 @@ +/** Converts a unified hunk side's start/count into its consumed-file range. */ +export function getHunkSideStartBoundary(start: number, count: number): number { + return start - (count === 0 ? 0 : 1); +} + +export function getHunkSideEndBoundary(start: number, count: number): number { + return getHunkSideStartBoundary(start, count) + count; +} diff --git a/packages/diffs/src/utils/getTotalLineCountFromHunks.ts b/packages/diffs/src/utils/getTotalLineCountFromHunks.ts index f6bfc946f..67bbc5105 100644 --- a/packages/diffs/src/utils/getTotalLineCountFromHunks.ts +++ b/packages/diffs/src/utils/getTotalLineCountFromHunks.ts @@ -1,4 +1,5 @@ import type { Hunk } from '../types'; +import { getHunkSideEndBoundary } from './getHunkSideBoundaries'; export function getTotalLineCountFromHunks(hunks: Hunk[]): number { const lastHunk = hunks.at(-1); @@ -6,7 +7,7 @@ export function getTotalLineCountFromHunks(hunks: Hunk[]): number { return 0; } return Math.max( - lastHunk.additionStart + lastHunk.additionCount, - lastHunk.deletionStart + lastHunk.deletionCount + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount), + getHunkSideEndBoundary(lastHunk.deletionStart, lastHunk.deletionCount) ); } diff --git a/packages/diffs/src/utils/hydratePartialDiff.ts b/packages/diffs/src/utils/hydratePartialDiff.ts index fa4c686c0..83955e300 100644 --- a/packages/diffs/src/utils/hydratePartialDiff.ts +++ b/packages/diffs/src/utils/hydratePartialDiff.ts @@ -5,6 +5,10 @@ import type { Hunk, } from '../types'; import { cloneFileDiffMetadata } from './cloneFileDiffMetadata'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; import { splitFileContents } from './splitFileContents'; interface HydratedHunksResult { @@ -122,7 +126,8 @@ function hydrateHunks( } const collapsedBefore = Math.max( - hunk.additionStart - 1 - lastHunkAdditionEnd, + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) - + lastHunkAdditionEnd, 0 ); hydratedHunks.push({ @@ -141,14 +146,17 @@ function hydrateHunks( splitLineCount += collapsedBefore + hunkSplitLineCount; unifiedLineCount += collapsedBefore + hunkUnifiedLineCount; - lastHunkAdditionEnd = hunk.additionStart + hunk.additionCount - 1; + lastHunkAdditionEnd = getHunkSideEndBoundary( + hunk.additionStart, + hunk.additionCount + ); } if (hydratedHunks.length > 0) { const lastHunk = hydratedHunks[hydratedHunks.length - 1]; - const lastHunkEnd = Math.max( - lastHunk.additionStart + lastHunk.additionCount - 1, - 0 + const lastHunkEnd = getHunkSideEndBoundary( + lastHunk.additionStart, + lastHunk.additionCount ); const collapsedAfter = Math.max(totalAdditionLines - lastHunkEnd, 0); splitLineCount += collapsedAfter; diff --git a/packages/diffs/src/utils/iterateOverDiff.ts b/packages/diffs/src/utils/iterateOverDiff.ts index cc53349cf..62a5fe1ac 100644 --- a/packages/diffs/src/utils/iterateOverDiff.ts +++ b/packages/diffs/src/utils/iterateOverDiff.ts @@ -5,6 +5,7 @@ import type { Hunk, HunkExpansionRegion, } from '../types'; +import { getHunkSideStartBoundary } from './getHunkSideBoundaries'; import { getExpandedRegion, getTrailingExpandedRegion, @@ -230,6 +231,23 @@ export function iterateOverDiff({ break; } + const deletionBoundary = getHunkSideStartBoundary( + hunk.deletionStart, + hunk.deletionCount + ); + const additionBoundary = getHunkSideStartBoundary( + hunk.additionStart, + hunk.additionCount + ); + const deletionStartIndex = + !diff.isPartial && hunk.deletionCount === 0 + ? deletionBoundary + : hunk.deletionLineIndex; + const additionStartIndex = + !diff.isPartial && hunk.additionCount === 0 + ? additionBoundary + : hunk.additionLineIndex; + const leadingRegion = getExpandedRegion({ isPartial: diff.isPartial, rangeSize: hunk.collapsedBefore, @@ -285,10 +303,10 @@ export function iterateOverDiff({ let unifiedLineIndex = hunk.unifiedLineStart - leadingRegion.rangeSize; let splitLineIndex = hunk.splitLineStart - leadingRegion.rangeSize; - let deletionLineIndex = hunk.deletionLineIndex - leadingRegion.rangeSize; - let additionLineIndex = hunk.additionLineIndex - leadingRegion.rangeSize; - let deletionLineNumber = hunk.deletionStart - leadingRegion.rangeSize; - let additionLineNumber = hunk.additionStart - leadingRegion.rangeSize; + let deletionLineIndex = deletionStartIndex - leadingRegion.rangeSize; + let additionLineIndex = additionStartIndex - leadingRegion.rangeSize; + let deletionLineNumber = deletionBoundary + 1 - leadingRegion.rangeSize; + let additionLineNumber = additionBoundary + 1 - leadingRegion.rangeSize; if ( walkContextLines(state, leadingRegion.fromStart, diffStyle, (index) => { @@ -321,10 +339,10 @@ export function iterateOverDiff({ unifiedLineIndex = hunk.unifiedLineStart - leadingRegion.fromEnd; splitLineIndex = hunk.splitLineStart - leadingRegion.fromEnd; - deletionLineIndex = hunk.deletionLineIndex - leadingRegion.fromEnd; - additionLineIndex = hunk.additionLineIndex - leadingRegion.fromEnd; - deletionLineNumber = hunk.deletionStart - leadingRegion.fromEnd; - additionLineNumber = hunk.additionStart - leadingRegion.fromEnd; + deletionLineIndex = deletionStartIndex - leadingRegion.fromEnd; + additionLineIndex = additionStartIndex - leadingRegion.fromEnd; + deletionLineNumber = deletionBoundary + 1 - leadingRegion.fromEnd; + additionLineNumber = additionBoundary + 1 - leadingRegion.fromEnd; if ( walkContextLines( state, @@ -371,10 +389,10 @@ export function iterateOverDiff({ let unifiedLineIndex = hunk.unifiedLineStart; let splitLineIndex = hunk.splitLineStart; - let deletionLineIndex = hunk.deletionLineIndex; - let additionLineIndex = hunk.additionLineIndex; - let deletionLineNumber = hunk.deletionStart; - let additionLineNumber = hunk.additionStart; + let deletionLineIndex = deletionStartIndex; + let additionLineIndex = additionStartIndex; + let deletionLineNumber = deletionBoundary + 1; + let additionLineNumber = additionBoundary + 1; const lastContent = hunk.hunkContent.at(-1); for (const content of hunk.hunkContent) { diff --git a/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts b/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts index c4438efbe..a1218458d 100644 --- a/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts +++ b/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts @@ -22,7 +22,7 @@ // Snapshot tests (from packages/diffs/): // bun test parseMergeConflictDiffFromFile // -// Performance benchmark (checksum must match 33121550): +// Performance benchmark (checksum must match 31314920): // moonx diffs:benchmark-parse-merge-conflict // // If you encounter a bug: @@ -47,6 +47,10 @@ import type { MergeConflictRegion, ProcessFileConflictData, } from '../types'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; export interface ParseMergeConflictDiffFromFileResult { fileDiff: FileDiffMetadata; @@ -275,7 +279,7 @@ export function parseMergeConflictDiffFromFile( const lastHunk = s.hunks[s.hunks.length - 1]; const collapsedAfter = Math.max( s.additionLines.length - - (lastHunk.additionStart + lastHunk.additionCount - 1), + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount), 0 ); s.splitLineCount += collapsedAfter; @@ -594,20 +598,33 @@ function finalizeActiveHunk(s: ParseState): void { } } - const collapsedBefore = Math.max(hunk.additionStart - 1 - s.lastHunkEnd, 0); + // While building, each side's start is "next line to consume" (1-based). A + // side that finished with zero lines must shift down to the unified `N,0` + // convention (start names the line the hunk applies after), which is what + // hunk headers, getHunkSideStartBoundary, and downstream boundary math all + // assume — e.g. a whole-file deletion emits `+0,0`, not `+1,0`. + const additionStart = + hunk.additionCount === 0 ? hunk.additionStart - 1 : hunk.additionStart; + const deletionStart = + hunk.deletionCount === 0 ? hunk.deletionStart - 1 : hunk.deletionStart; + + const collapsedBefore = Math.max( + getHunkSideStartBoundary(additionStart, hunk.additionCount) - s.lastHunkEnd, + 0 + ); const finalizedHunk: Hunk = { collapsedBefore, - additionStart: hunk.additionStart, + additionStart, additionCount: hunk.additionCount, additionLines: hunk.additionLines, additionLineIndex: hunk.additionLineIndex, - deletionStart: hunk.deletionStart, + deletionStart, deletionCount: hunk.deletionCount, deletionLines: hunk.deletionLines, deletionLineIndex: hunk.deletionLineIndex, hunkContent: hunk.hunkContent, hunkContext: undefined, - hunkSpecs: `@@ -${formatHunkRange(hunk.deletionStart, hunk.deletionCount)} +${formatHunkRange(hunk.additionStart, hunk.additionCount)} @@\n`, + hunkSpecs: `@@ -${formatHunkRange(deletionStart, hunk.deletionCount)} +${formatHunkRange(additionStart, hunk.additionCount)} @@\n`, splitLineStart: s.splitLineCount + collapsedBefore, splitLineCount: hunkSplitLineCount, unifiedLineStart: s.unifiedLineCount + collapsedBefore, @@ -619,7 +636,7 @@ function finalizeActiveHunk(s: ParseState): void { s.hunks.push(finalizedHunk); s.splitLineCount += collapsedBefore + hunkSplitLineCount; s.unifiedLineCount += collapsedBefore + hunkUnifiedLineCount; - s.lastHunkEnd = hunk.additionStart + hunk.additionCount - 1; + s.lastHunkEnd = getHunkSideEndBoundary(additionStart, hunk.additionCount); } // Called when the context buffer between two changes exceeds maxContextLines*2. diff --git a/packages/diffs/src/utils/parsePatchFiles.ts b/packages/diffs/src/utils/parsePatchFiles.ts index 1f52141e1..da54f13f6 100644 --- a/packages/diffs/src/utils/parsePatchFiles.ts +++ b/packages/diffs/src/utils/parsePatchFiles.ts @@ -17,6 +17,10 @@ import type { } from '../types'; import { cleanLastNewline } from './cleanLastNewline'; import { detachString, releaseStringDetachBuffer } from './detachString'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; import { realignChangeContentBySimilarity } from './realignChangeContent'; interface ParsedHunkHeader { @@ -489,11 +493,15 @@ function _processFile( hunkData.deletionLines = deletionLines; hunkData.collapsedBefore = Math.max( - hunkData.additionStart - 1 - lastHunkEnd, + getHunkSideStartBoundary(hunkData.additionStart, hunkData.additionCount) - + lastHunkEnd, 0 ); currentFile.hunks.push(hunkData); - lastHunkEnd = hunkData.additionStart + hunkData.additionCount - 1; + lastHunkEnd = getHunkSideEndBoundary( + hunkData.additionStart, + hunkData.additionCount + ); for (const content of hunkData.hunkContent) { if (content.type === 'context') { hunkData.splitLineCount += content.lines; @@ -537,7 +545,10 @@ function _processFile( currentFile.deletionLines.length > 0 ) { const lastHunk = currentFile.hunks[currentFile.hunks.length - 1]; - const lastHunkEnd = lastHunk.additionStart + lastHunk.additionCount - 1; + const lastHunkEnd = getHunkSideEndBoundary( + lastHunk.additionStart, + lastHunk.additionCount + ); const totalFileLines = currentFile.additionLines.length; const collapsedAfter = Math.max(totalFileLines - lastHunkEnd, 0); currentFile.splitLineCount += collapsedAfter; diff --git a/packages/diffs/src/utils/realignChangeContent.ts b/packages/diffs/src/utils/realignChangeContent.ts index fa3a6d6fc..4f26e5e88 100644 --- a/packages/diffs/src/utils/realignChangeContent.ts +++ b/packages/diffs/src/utils/realignChangeContent.ts @@ -59,11 +59,16 @@ export function realignChangeContentBySimilarity( * the run's bottom — so pressing Enter at the end of a line marks a blank * *below* the caret as inserted while the caret's own new line renders as * context. Sliding up anchors the change to the content above it (the caret - * line after an Enter) instead. Non-blank blocks never slide, so code that - * merely ends like its neighbor (an added function before an identical `}`) - * keeps the library's canonical position. Runs on final assembled - * hunkContent: from the parse post-pass above and from the edit session's - * region re-diff, so mid-session and exit renderings agree. + * line after an Enter) instead. + * + * The slide is all-or-nothing: it only applies when the block comes to rest + * directly beneath remaining in-hunk content. A slide that would consume the + * hunk's entire leading context was stopped by the hunk's edge — a context + * window cut, not the top of the blank run — and that landing spot is + * arbitrary, so the block keeps the library's bottom-of-run anchor (which + * sits against the content below the run). Non-blank blocks never slide, so + * code that merely ends like its neighbor (an added function before an + * identical `}`) keeps the library's canonical position. */ export function slideBlankBoundaryBlocksUp( hunk: Hunk, @@ -119,6 +124,12 @@ export function slideBlankBoundaryBlocksUp( if (slide === 0) { continue; } + // Stopped by the hunk's leading edge rather than by content: the blank + // run extends to (and possibly beyond) the hunk's top, so there is no + // in-hunk line to anchor beneath. Keep the bottom-of-run position. + if (index === 1 && slide === previous.lines) { + continue; + } block.additionLineIndex -= slide; block.deletionLineIndex -= slide; diff --git a/packages/diffs/src/utils/resolveRegion.ts b/packages/diffs/src/utils/resolveRegion.ts index 3b9783c0e..0776d617e 100644 --- a/packages/diffs/src/utils/resolveRegion.ts +++ b/packages/diffs/src/utils/resolveRegion.ts @@ -4,6 +4,10 @@ import type { FileDiffMetadata, Hunk, } from '../types'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; interface RegionResolutionTarget { hunkIndex: number; @@ -81,8 +85,10 @@ export function resolveRegion( diff, resolvedDiff, cursor, - hunk.deletionLineIndex - hunk.collapsedBefore, - hunk.additionLineIndex - hunk.collapsedBefore, + getHunkSideStartBoundary(hunk.deletionStart, hunk.deletionCount) - + hunk.collapsedBefore, + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) - + hunk.collapsedBefore, hunk.collapsedBefore, shouldProcessCollapsedContext ); @@ -185,24 +191,46 @@ export function resolveRegion( newHunk.noEOFCRDeletions = noEOFCR; } + if (newHunk.additionCount === 0) { + newHunk.additionStart--; + if (!diff.isPartial) { + newHunk.additionLineIndex--; + } + } + if (newHunk.deletionCount === 0) { + newHunk.deletionStart--; + if (!diff.isPartial) { + newHunk.deletionLineIndex--; + } + } + resolvedDiff.hunks.push(newHunk); } const finalHunk = hunks.at(-1); if (finalHunk != null && !diff.isPartial) { + const finalDeletionEnd = getHunkSideEndBoundary( + finalHunk.deletionStart, + finalHunk.deletionCount + ); + const finalAdditionEnd = getHunkSideEndBoundary( + finalHunk.additionStart, + finalHunk.additionCount + ); + const trailingContext = Math.min( + deletionLines.length - finalDeletionEnd, + additionLines.length - finalAdditionEnd + ); pushCollapsedContextLines( resolvedDiff, deletionLines, additionLines, - finalHunk.deletionLineIndex + finalHunk.deletionCount, - finalHunk.additionLineIndex + finalHunk.additionCount, - Math.min( - deletionLines.length - - (finalHunk.deletionLineIndex + finalHunk.deletionCount), - additionLines.length - - (finalHunk.additionLineIndex + finalHunk.additionCount) - ) + finalDeletionEnd, + finalAdditionEnd, + trailingContext ); + cursor.splitLineCount += trailingContext; + cursor.unifiedLineCount += trailingContext; } resolvedDiff.splitLineCount = cursor.splitLineCount; diff --git a/packages/diffs/src/utils/updateDiffHunks.ts b/packages/diffs/src/utils/updateDiffHunks.ts index d001fc130..6d1aeb234 100644 --- a/packages/diffs/src/utils/updateDiffHunks.ts +++ b/packages/diffs/src/utils/updateDiffHunks.ts @@ -7,6 +7,10 @@ import type { Hunk, } from '../types'; import { cleanLastNewline } from './cleanLastNewline'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; import { parseDiffFromFile } from './parseDiffFromFile'; import { hasTrailingContextMismatch } from './virtualDiffLayout'; @@ -188,8 +192,10 @@ export function preserveTrailingEditorBlankLine( return; } - const lastHunkAdditionEnd = - lastHunk.additionLineIndex + lastHunk.additionCount; + const lastHunkAdditionEnd = getHunkSideEndBoundary( + lastHunk.additionStart, + lastHunk.additionCount + ); if (lastHunkAdditionEnd !== extraAdditionLineIndex) { return; } @@ -463,7 +469,8 @@ export function recomputeDiffRenderLineCounts( for (const hunk of diff.hunks) { hunk.collapsedBefore = Math.max( - hunk.additionStart - 1 - lastHunkAdditionEnd, + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) - + lastHunkAdditionEnd, 0 ); hunk.splitLineStart = splitTotal + hunk.collapsedBefore; @@ -473,14 +480,17 @@ export function recomputeDiffRenderLineCounts( splitTotal += hunk.collapsedBefore + hunk.splitLineCount; unifiedTotal += hunk.collapsedBefore + hunk.unifiedLineCount; - lastHunkAdditionEnd = hunk.additionStart + hunk.additionCount - 1; + lastHunkAdditionEnd = getHunkSideEndBoundary( + hunk.additionStart, + hunk.additionCount + ); } if (diff.hunks.length > 0) { const lastHunk = diff.hunks[diff.hunks.length - 1]; const collapsedAfter = Math.max( diff.additionLines.length - - (lastHunk.additionLineIndex + lastHunk.additionCount), + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount), 0 ); splitTotal += collapsedAfter; diff --git a/packages/diffs/src/utils/virtualDiffLayout.ts b/packages/diffs/src/utils/virtualDiffLayout.ts index 379373c4b..1a61f02a9 100644 --- a/packages/diffs/src/utils/virtualDiffLayout.ts +++ b/packages/diffs/src/utils/virtualDiffLayout.ts @@ -1,10 +1,15 @@ import type { FileDiffMetadata, + Hunk, HunkExpansionRegion, HunkSeparators, VirtualFileMetrics, } from '../types'; import { getDefaultHunkSeparatorHeight } from './computeVirtualFileMetrics'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; export interface ExpandedRegionResult { fromStart: number; @@ -33,6 +38,14 @@ export interface GetTrailingExpandedRegionProps extends GetTrailingContextRangeS collapsedContextThreshold: number; } +/** One-based new-file line range covered by a hunk, as `[start, end)`. */ +export function getHunkAdditionLineRange(hunk: Hunk): [number, number] { + return [ + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) + 1, + getHunkSideEndBoundary(hunk.additionStart, hunk.additionCount) + 1, + ]; +} + export interface HunkSeparatorLayout { height: number; gapBefore: number; @@ -116,10 +129,10 @@ export function hasTrailingContext(fileDiff: FileDiffMetadata): boolean { const additionRemaining = fileDiff.additionLines.length - - (lastHunk.additionLineIndex + lastHunk.additionCount); + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount); const deletionRemaining = fileDiff.deletionLines.length - - (lastHunk.deletionLineIndex + lastHunk.deletionCount); + getHunkSideEndBoundary(lastHunk.deletionStart, lastHunk.deletionCount); return additionRemaining > 0 || deletionRemaining > 0; } @@ -140,10 +153,10 @@ export function hasTrailingContextMismatch( const additionRemaining = fileDiff.additionLines.length - - (lastHunk.additionLineIndex + lastHunk.additionCount); + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount); const deletionRemaining = fileDiff.deletionLines.length - - (lastHunk.deletionLineIndex + lastHunk.deletionCount); + getHunkSideEndBoundary(lastHunk.deletionStart, lastHunk.deletionCount); if (additionRemaining <= 0 && deletionRemaining <= 0) { return false; @@ -170,10 +183,10 @@ export function getTrailingContextRangeSize({ const additionRemaining = fileDiff.additionLines.length - - (lastHunk.additionLineIndex + lastHunk.additionCount); + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount); const deletionRemaining = fileDiff.deletionLines.length - - (lastHunk.deletionLineIndex + lastHunk.deletionCount); + getHunkSideEndBoundary(lastHunk.deletionStart, lastHunk.deletionCount); if (additionRemaining <= 0 && deletionRemaining <= 0) { return 0; @@ -262,7 +275,8 @@ export function isAdditionLineRenderable({ } for (const [hunkIndex, hunk] of fileDiff.hunks.entries()) { - if (lineNumber < hunk.additionStart) { + const [hunkStart, hunkEnd] = getHunkAdditionLineRange(hunk); + if (lineNumber < hunkStart) { // Inside the collapsed gap before this hunk: renderable only within // the expanded slices at the gap's edges. const region = getExpandedRegion({ @@ -272,14 +286,14 @@ export function isAdditionLineRenderable({ hunkIndex, collapsedContextThreshold, }); - const gapStart = hunk.additionStart - region.rangeSize; + const gapStart = hunkStart - region.rangeSize; return ( region.renderAll || lineNumber < gapStart + region.fromStart || - lineNumber >= hunk.additionStart - region.fromEnd + lineNumber >= hunkStart - region.fromEnd ); } - if (lineNumber < hunk.additionStart + hunk.additionCount) { + if (lineNumber < hunkEnd) { return true; } } @@ -295,7 +309,7 @@ export function isAdditionLineRenderable({ return true; } const lastHunk = fileDiff.hunks[fileDiff.hunks.length - 1]; - const trailingStart = lastHunk.additionStart + lastHunk.additionCount; + const [, trailingStart] = getHunkAdditionLineRange(lastHunk); return ( lineNumber < trailingStart + trailingRegion.fromStart || lineNumber >= trailingStart + trailingRegion.rangeSize @@ -330,6 +344,7 @@ export function getNearestRenderableAdditionLine({ const ranges: Array<[start: number, end: number]> = []; let modeledEnd = 1; for (const [hunkIndex, hunk] of fileDiff.hunks.entries()) { + const [hunkStart, hunkEnd] = getHunkAdditionLineRange(hunk); const region = getExpandedRegion({ isPartial: fileDiff.isPartial, rangeSize: hunk.collapsedBefore, @@ -337,19 +352,19 @@ export function getNearestRenderableAdditionLine({ hunkIndex, collapsedContextThreshold, }); - const gapStart = hunk.additionStart - region.rangeSize; + const gapStart = hunkStart - region.rangeSize; if (region.renderAll) { - ranges.push([gapStart, hunk.additionStart]); + ranges.push([gapStart, hunkStart]); } else { if (region.fromStart > 0) { ranges.push([gapStart, gapStart + region.fromStart]); } if (region.fromEnd > 0) { - ranges.push([hunk.additionStart - region.fromEnd, hunk.additionStart]); + ranges.push([hunkStart - region.fromEnd, hunkStart]); } } - ranges.push([hunk.additionStart, hunk.additionStart + hunk.additionCount]); - modeledEnd = hunk.additionStart + hunk.additionCount; + ranges.push([hunkStart, hunkEnd]); + modeledEnd = hunkEnd; } const trailingRegion = getTrailingExpandedRegion({ fileDiff, diff --git a/packages/diffs/test/CodeView.rangeScroll.test.ts b/packages/diffs/test/CodeView.rangeScroll.test.ts index bb7cbe96d..a477fd9a0 100644 --- a/packages/diffs/test/CodeView.rangeScroll.test.ts +++ b/packages/diffs/test/CodeView.rangeScroll.test.ts @@ -44,6 +44,24 @@ function makeInsertedDiffItem(id: string): CodeViewItem { }; } +function makeDeletedDiffItem(id: string): CodeViewItem { + const oldLines = Array.from( + { length: 160 }, + (_, index) => `line ${index + 1}` + ); + const newLines = oldLines.toSpliced(80, 1); + + return { + id, + type: 'diff', + fileDiff: parseDiffFromFile( + { name: 'src/deleted.ts', contents: oldLines.join('\n') }, + { name: 'src/deleted.ts', contents: newLines.join('\n') }, + { context: 0 } + ), + }; +} + function getFileLineTop(lineNumber: number): number { return ( DEFAULT_CODE_VIEW_FILE_METRICS.diffHeaderHeight + @@ -260,4 +278,44 @@ describe('CodeView range scrolling', () => { cleanup(); } }); + + test('places a context-zero deletion between adjacent addition-side lines', async () => { + const { cleanup } = installDom(); + const viewer = new CodeView({ diffStyle: 'split', expandUnchanged: true }); + const root = createRoot({ height: ROOT_HEIGHT, width: ROOT_WIDTH }); + + try { + viewer.setup(root); + await renderItems(viewer, [makeDeletedDiffItem('diff:deleted')]); + + viewer.scrollTo({ + type: 'line', + id: 'diff:deleted', + lineNumber: 80, + side: 'additions', + align: 'center', + behavior: 'instant', + }); + viewer.render(true); + const beforeDeletionScrollTop = root.scrollTop; + + viewer.scrollTo({ + type: 'line', + id: 'diff:deleted', + lineNumber: 81, + side: 'additions', + align: 'center', + behavior: 'instant', + }); + viewer.render(true); + + expect(root.scrollTop - beforeDeletionScrollTop).toBe( + 2 * DEFAULT_CODE_VIEW_FILE_METRICS.lineHeight + ); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); }); diff --git a/packages/diffs/test/DiffHunksRendererRecompute.test.ts b/packages/diffs/test/DiffHunksRendererRecompute.test.ts index 239bfcf8b..28a55e3e9 100644 --- a/packages/diffs/test/DiffHunksRendererRecompute.test.ts +++ b/packages/diffs/test/DiffHunksRendererRecompute.test.ts @@ -10,6 +10,11 @@ import type { FileDiffMetadata, HighlightedToken } from '../src/types'; import type { DiffsTextDocument } from '../src/types'; import { finishEditSessionForDiff } from '../src/utils/editSessionHunks'; import { iterateOverDiff } from '../src/utils/iterateOverDiff'; +import { + collectAllElements, + hastTextContent, + projectRenderResult, +} from './testUtils'; afterAll(async () => { await disposeHighlighter(); @@ -68,6 +73,25 @@ function makeTextDocument(lines: string[]): DiffsTextDocument { }; } +function pairingProjection(diff: FileDiffMetadata) { + return diff.hunks.flatMap((hunk) => + hunk.hunkContent.flatMap((content) => { + if (content.type !== 'change') return []; + return Array.from( + { length: Math.max(content.deletions, content.additions) }, + (_, offset) => [ + offset < content.deletions + ? content.deletionLineIndex + offset + : undefined, + offset < content.additions + ? content.additionLineIndex + offset + : undefined, + ] + ); + }) + ); +} + // Builds a renderer with a populated (highlighted) render cache, mirroring the // state the editor operates on mid-session. async function createPrimedRenderer( @@ -229,10 +253,116 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { ); await renderer.asyncRender(diff); renderer.renderDiff(diff); - renderer.beginEditSession(diff); + renderer.beginEditSession(); return { renderer, diff }; } + test('a mid-document insertion realigns cached addition rows', async () => { + // Cached per-line HAST is looked up by line index. A run of identical + // blank lines with `last` at the bottom hidden behind collapsed context: + // inserting a line above shifts every following index, and the collapsed + // rows only become visible after session exit — if the cache is not + // realigned they render another line's stale tokens (a duplicated + // `last` in the playground). + const oldContents = 'first\n' + '\n'.repeat(9) + 'last\n'; + const newContents = 'changed\n' + '\n'.repeat(9) + 'last\n'; + const renderer = new DiffHunksRenderer({ + theme: 'github-light', + diffStyle: 'split', + }); + const diff = parseDiffFromFile( + { name: 'jump.ts', contents: oldContents, cacheKey: 'jump:old' }, + { name: 'jump.ts', contents: newContents, cacheKey: 'jump:new' } + ); + await renderer.asyncRender(diff); + renderer.renderDiff(diff); + renderer.beginEditSession(); + + // The Enter keystroke: one blank line inserted below the changed line. + const editedLines = newContents.split('\n'); + editedLines.splice(1, 0, ''); + renderer.applyDocumentChange(makeTextDocument(editedLines)); + + renderer.endEditSession(); + finishEditSessionForDiff(diff); + + const result = renderer.renderDiff(diff); + expect(result).toBeDefined(); + if (result == null) return; + const rows = (projectRenderResult(result).additions ?? []).filter( + (row) => row.kind === 'line' + ); + // Every rendered addition row must show its own source line. + const mismatches = rows.filter( + (row) => + row.lineNumber != null && + row.text !== diff.additionLines[row.lineNumber - 1].replace(/\n$/, '') + ); + expect(mismatches.map((row) => `#${row.lineNumber}: ${row.text}`)).toEqual( + [] + ); + expect( + rows.filter((row) => row.text === 'last').map((row) => row.lineNumber) + ).toEqual([12]); + }); + + test('session exit restores highlighting for realigned rows', async () => { + // Realignment plain-fills lines inside the changed window (their old + // slots were legitimately rewritten mid-pass), and hidden rows are never + // re-tokenized by the editor. Refreshing the highlighted result at + // exit — as FileDiff.completeEditSession does — must restore full + // highlighting without ever rendering the interim view unhighlighted. + const trailing = 'const last = true;'; + const oldContents = 'first\n' + '\n'.repeat(9) + trailing + '\n'; + const newContents = 'changed\n' + '\n'.repeat(9) + trailing + '\n'; + const renderer = new DiffHunksRenderer({ + theme: 'github-light', + diffStyle: 'split', + }); + const diff = parseDiffFromFile( + { name: 'jump.ts', contents: oldContents, cacheKey: 'jump:old' }, + { name: 'jump.ts', contents: newContents, cacheKey: 'jump:new' } + ); + await renderer.asyncRender(diff); + renderer.renderDiff(diff); + renderer.beginEditSession(); + + const editedLines = newContents.split('\n'); + // Mirror the dirty-token pass that precedes applyDocumentChange: the + // tokenizer rewrites the shifted trailing line's slot (old index 10) + // with its post-edit content, which is what strands the moved line in + // the realign's plain-filled window. + renderer.updateRenderCache(makeDirtyLines([[10, '']]), 'light', true); + editedLines.splice(1, 0, ''); + renderer.applyDocumentChange(makeTextDocument(editedLines)); + + renderer.endEditSession(); + finishEditSessionForDiff(diff); + const refresh = renderer.refreshHighlightedResult(); + + // Highlighted rows carry color styles on their token spans; the + // realign's plain-filled element has none. + const styledRowTexts = (result: ReturnType) => + collectAllElements(result?.additionsContentAST ?? []) + .filter( + (node) => + node.properties?.['data-line'] != null && + JSON.stringify(node).includes('color:') + ) + .map((node) => hastTextContent(node).replace(/\n$/, '')); + + // The exit repaint runs before the fresh highlight lands: it must keep + // serving the current result (no un-highlighted flash), with only the + // realigned window plain. + expect(styledRowTexts(renderer.renderDiff(diff))).toEqual(['changed']); + + await refresh; + expect(styledRowTexts(renderer.renderDiff(diff))).toEqual([ + 'changed', + trailing, + ]); + }); + test('reverting a hunk keeps it as a context-only region', async () => { const { renderer, diff } = await createSessionRenderer(); const boundsBefore = diff.hunks.map((hunk) => ({ @@ -269,10 +399,69 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { expect(diff.hunks[1].additionLineIndex).toBe(11); }); - // The regression guard for the same-pass contamination trap: a line-count - // pass tokenizes every shifted line as dirty at post-edit indexes while - // diff.additionLines still holds pre-edit content. Region work must wait - // for applyDocumentChange, which scans against the pass snapshot. + test('reports a row-topology change when region bounds stay fixed', async () => { + const renderer = new DiffHunksRenderer({ + theme: 'github-light', + diffStyle: 'split', + }); + const diff = parseDiffFromFile( + { name: 'topology.ts', contents: 'a\nb\nc\nd\n' }, + { name: 'topology.ts', contents: 'a\na\na\nd\n' } + ); + await renderer.asyncRender(diff); + renderer.renderDiff(diff); + renderer.beginEditSession(); + const boundsBefore = { + additionLineIndex: diff.hunks[0].additionLineIndex, + additionCount: diff.hunks[0].additionCount, + deletionLineIndex: diff.hunks[0].deletionLineIndex, + deletionCount: diff.hunks[0].deletionCount, + }; + const splitCountBefore = diff.hunks[0].splitLineCount; + + const regionsChanged = renderer.updateRenderCache( + makeDirtyLines([[2, 'b']]), + 'light' + ); + + expect(regionsChanged).toBe(true); + expect(diff.hunks[0]).toMatchObject(boundsBefore); + expect(diff.hunks[0].splitLineCount).not.toBe(splitCountBefore); + }); + + test('reports a changed split pairing when the row count stays fixed', async () => { + const renderer = new DiffHunksRenderer({ + theme: 'github-light', + diffStyle: 'split', + }); + const diff = parseDiffFromFile( + { name: 'pairing.ts', contents: '\nb\nb\nc\n' }, + { name: 'pairing.ts', contents: '\n!\nb\nc\n' } + ); + await renderer.asyncRender(diff); + renderer.renderDiff(diff); + renderer.beginEditSession(); + const boundsBefore = { + additionLineIndex: diff.hunks[0].additionLineIndex, + additionCount: diff.hunks[0].additionCount, + deletionLineIndex: diff.hunks[0].deletionLineIndex, + deletionCount: diff.hunks[0].deletionCount, + }; + const splitCountBefore = diff.hunks[0].splitLineCount; + + const regionsChanged = renderer.updateRenderCache( + makeDirtyLines([[0, 'b']]), + 'light' + ); + + expect(regionsChanged).toBe(true); + expect(diff.hunks[0]).toMatchObject(boundsBefore); + expect(diff.hunks[0].splitLineCount).toBe(splitCountBefore); + }); + + // A line-count pass tokenizes every shifted line as dirty at post-edit + // indexes while diff.additionLines still holds pre-edit content. Region work + // must wait for applyDocumentChange's authoritative document rebuild. test('an Enter keystroke does not disturb other regions', async () => { const { renderer, diff } = await createSessionRenderer(); const secondRegionBefore = { @@ -309,6 +498,27 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { expect(diff.hunks[1].deletionCount).toBe(secondRegionBefore.deletionCount); expect(diff.hunks[1].additionCount).toBe(secondRegionBefore.additionCount); expect(diff.additionLines.join('')).toBe(postEditLines.join('')); + const full = parseDiffFromFile( + { name: 'session.ts', contents: SESSION_OLD.join('') }, + { name: 'session.ts', contents: postEditLines.join('') } + ); + expect(pairingProjection(diff)).toEqual(pairingProjection(full)); + }); + + test('a same-line edit can rebuild after preserving the trailing editor row', async () => { + const { renderer, diff } = await createSessionRenderer(); + renderer.applyDocumentChange(makeTextDocumentFromText('line 1\n')); + expect(diff.additionLines).toEqual(['line 1\n', '']); + + expect(() => + renderer.updateRenderCache( + makeDirtyLines([[0, 'line 1 edited']]), + 'light' + ) + ).not.toThrow(); + + expect(diff.additionLines).toEqual(['line 1 edited\n', '']); + expect(renderer.renderDiff()).toBeDefined(); }); test('a blank line pushed above an edited line keeps the pair aligned', async () => { @@ -381,6 +591,37 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { expect(renderer.renderFullHTML(result)).toContain('change-addition'); }); + test('a same-line undo back to empty reapplies the empty-document shim', async () => { + const { renderer, diff } = await createSessionRenderer(); + renderer.applyDocumentChange(makeTextDocument([''])); + renderer.updateRenderCache(makeDirtyLines([[0, 'hello']]), 'light'); + + const regionsChanged = renderer.updateRenderCache( + makeDirtyLines([[0, '']]), + 'light' + ); + + expect(regionsChanged).toBe(true); + expect(diff.additionLines).toEqual(['']); + const result = renderer.renderDiff(); + expect(result).toBeDefined(); + if (result == null) return; + expect(renderer.renderFullHTML(result)).toContain('change-addition'); + }); + + test('typing into a newline-only document rebuilds from canonical lines', async () => { + const { renderer, diff } = await createSessionRenderer(); + renderer.applyDocumentChange(makeTextDocumentFromText('\n')); + expect(diff.additionLines).toEqual(['\n', '']); + + expect(() => + renderer.updateRenderCache(makeDirtyLines([[0, 'typed']]), 'light') + ).not.toThrow(); + + expect(diff.additionLines).toEqual(['typed\n', '']); + expect(renderer.renderDiff()).toBeDefined(); + }); + test('genuine session end recomputes; a zero-edit session does not', async () => { const { renderer, diff } = await createSessionRenderer(); const untouchedHunks = diff.hunks; diff --git a/packages/diffs/test/diffAcceptRejectHunk.test.ts b/packages/diffs/test/diffAcceptRejectHunk.test.ts index 20692036f..2ead51c3f 100644 --- a/packages/diffs/test/diffAcceptRejectHunk.test.ts +++ b/packages/diffs/test/diffAcceptRejectHunk.test.ts @@ -286,6 +286,50 @@ function assertResolvedHunkMatchesExpected( } describe('diffAcceptRejectHunk', () => { + test('accepts a context-zero deletion without losing trailing context', () => { + const oldLines = Array.from( + { length: 7 }, + (_, index) => `line ${index + 1}\n` + ); + const newLines = oldLines.toSpliced(4, 1); + const diff = parseDiffFromFile( + { name: 'deletion.ts', contents: oldLines.join('') }, + { name: 'deletion.ts', contents: newLines.join('') }, + { context: 0 } + ); + + const result = diffAcceptRejectHunk(diff, 0, 'accept'); + + expect(result.deletionLines).toEqual(newLines); + expect(result.additionLines).toEqual(newLines); + expect(verifyFileDiffHunkValues(result)).toEqual({ + valid: true, + errors: [], + }); + }); + + test('rejects a context-zero deletion without duplicating trailing context', () => { + const oldLines = Array.from( + { length: 7 }, + (_, index) => `line ${index + 1}\n` + ); + const newLines = oldLines.toSpliced(4, 1); + const diff = parseDiffFromFile( + { name: 'deletion.ts', contents: oldLines.join('') }, + { name: 'deletion.ts', contents: newLines.join('') }, + { context: 0 } + ); + + const result = diffAcceptRejectHunk(diff, 0, 'reject'); + + expect(result.deletionLines).toEqual(oldLines); + expect(result.additionLines).toEqual(oldLines); + expect(verifyFileDiffHunkValues(result)).toEqual({ + valid: true, + errors: [], + }); + }); + test('accept keeps later hunk indices accurate after resolving a pure-addition hunk', () => { const diff = createFixture(); const resolvedSnapshot = snapshotHunk(diff, 0); diff --git a/packages/diffs/test/editSessionHunks.test.ts b/packages/diffs/test/editSessionHunks.test.ts index 7e67c0253..8a66b01fd 100644 --- a/packages/diffs/test/editSessionHunks.test.ts +++ b/packages/diffs/test/editSessionHunks.test.ts @@ -1,22 +1,22 @@ import { describe, expect, test } from 'bun:test'; +import type { CreatePatchOptionsNonabortable } from 'diff'; import type { FileDiffMetadata, HunkExpansionRegion } from '../src/types'; import { applySessionChangedLines, - applySessionEditWindow, captureExpansionAnchors, - findChangedLineWindow, + findDivergenceCore, finishEditSessionForDiff, normalizeEditorLines, rebuildExpansionFromAnchors, + rebuildSessionHunks, remapExpandedHunksForRegionChange, } from '../src/utils/editSessionHunks'; import { iterateOverDiff } from '../src/utils/iterateOverDiff'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { getTrailingContextRangeSize } from '../src/utils/virtualDiffLayout'; +import { verifyFileDiffHunkValues } from './testUtils'; -// 30-line file with two separated changes -> two hunks with a collapsible -// unchanged gap between them and trailing unchanged context after the second. function makeLines( count: number, edits: Record = {} @@ -45,6 +45,10 @@ function hunkBounds(diff: FileDiffMetadata) { })); } +function oldBounds(diff: FileDiffMetadata) { + return diff.hunks.map((hunk) => [hunk.deletionLineIndex, hunk.deletionCount]); +} + function countRenderedRows(diff: FileDiffMetadata): number { let rows = 0; iterateOverDiff({ @@ -58,6 +62,57 @@ function countRenderedRows(diff: FileDiffMetadata): number { return rows; } +function pairingProjection(diff: FileDiffMetadata) { + const rows: Array<{ + deletionIndex: number | undefined; + deletionText: string | undefined; + additionIndex: number | undefined; + additionText: string | undefined; + }> = []; + for (const hunk of diff.hunks) { + for (const content of hunk.hunkContent) { + if (content.type !== 'change') continue; + const rowCount = Math.max(content.deletions, content.additions); + for (let offset = 0; offset < rowCount; offset++) { + const deletionIndex = + offset < content.deletions + ? content.deletionLineIndex + offset + : undefined; + const additionIndex = + offset < content.additions + ? content.additionLineIndex + offset + : undefined; + rows.push({ + deletionIndex, + deletionText: + deletionIndex == null + ? undefined + : diff.deletionLines[deletionIndex], + additionIndex, + additionText: + additionIndex == null + ? undefined + : diff.additionLines[additionIndex], + }); + } + } + } + return rows; +} + +function expectPairingParity( + diff: FileDiffMetadata, + parseDiffOptions?: CreatePatchOptionsNonabortable +): void { + const expected = parseDiffFromFile( + { name: diff.prevName ?? diff.name, contents: diff.deletionLines.join('') }, + { name: diff.name, contents: diff.additionLines.join(''), lang: diff.lang }, + parseDiffOptions + ); + expect(pairingProjection(diff)).toEqual(pairingProjection(expected)); + expect(verifyFileDiffHunkValues(diff)).toEqual({ valid: true, errors: [] }); +} + describe('normalizeEditorLines', () => { test('drops only the phantom trailing empty line', () => { expect(normalizeEditorLines(['a\n', 'b\n', ''])).toEqual(['a\n', 'b\n']); @@ -66,188 +121,168 @@ describe('normalizeEditorLines', () => { }); }); -describe('findChangedLineWindow', () => { - test('locates a replaced line', () => { - expect( - findChangedLineWindow(['a\n', 'b\n', 'c\n'], ['a\n', 'x\n', 'c\n']) - ).toEqual({ start: 1, prevEnd: 2, nextEnd: 2 }); - }); - - test('locates a pure insert', () => { +describe('findDivergenceCore', () => { + test('finds replacement, insertion, and the identical case', () => { expect( - findChangedLineWindow(['a\n', 'c\n'], ['a\n', 'b\n', 'c\n']) - ).toEqual({ start: 1, prevEnd: 1, nextEnd: 2 }); - }); - - test('returns undefined for identical lines', () => { - expect(findChangedLineWindow(['a\n'], ['a\n'])).toBeUndefined(); + findDivergenceCore(['a\n', 'b\n', 'c\n'], ['a\n', 'x\n', 'c\n']) + ).toEqual({ start: 1, deletionEnd: 2, additionEnd: 2 }); + expect(findDivergenceCore(['a\n', 'c\n'], ['a\n', 'b\n', 'c\n'])).toEqual({ + start: 1, + deletionEnd: 1, + additionEnd: 2, + }); + expect(findDivergenceCore(['a\n'], ['a\n'])).toBeUndefined(); }); }); -describe('applySessionEditWindow', () => { - test('re-diff inside a region keeps boundaries and reports no change', () => { +describe('rebuildSessionHunks', () => { + test('derives downstream coordinates without moving old-side boundaries', () => { const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); - diff.additionLines[2] = 'changed again\n'; + const before = oldBounds(diff); + diff.additionLines.splice(3, 0, 'inserted\n'); - const change = applySessionEditWindow(diff, { - start: 2, - prevEnd: 3, - nextEnd: 3, - }); + const change = rebuildSessionHunks(diff); - expect(change).toBeUndefined(); - expect(hunkBounds(diff)).toEqual(boundsBefore); - expect(diff.editSessionDirty).toBe(true); + expect(change).toBeDefined(); + expect(oldBounds(diff)).toEqual(before); + expect(diff.hunks[0].additionCount).toBe(8); + expect(diff.hunks[1].additionLineIndex).toBe(16); + expect(() => + getTrailingContextRangeSize({ fileDiff: diff, errorPrefix: 'test' }) + ).not.toThrow(); + expectPairingParity(diff); }); - test('a reverted region persists as a context-only hunk', () => { + test('keeps a reverted region as context until session exit', () => { const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); + const before = hunkBounds(diff); const rowsBefore = countRenderedRows(diff); diff.additionLines[2] = 'l3\n'; - const change = applySessionEditWindow(diff, { - start: 2, - prevEnd: 3, - nextEnd: 3, - }); + expect(rebuildSessionHunks(diff)).toBeUndefined(); - expect(change).toBeUndefined(); - expect(diff.hunks).toHaveLength(2); - expect(hunkBounds(diff)).toEqual(boundsBefore); + expect(hunkBounds(diff)).toEqual(before); expect(diff.hunks[0].hunkContent).toEqual([ { type: 'context', - lines: boundsBefore[0].additionCount, - additionLineIndex: boundsBefore[0].additionLineIndex, - deletionLineIndex: boundsBefore[0].deletionLineIndex, + lines: before[0].additionCount, + additionLineIndex: before[0].additionLineIndex, + deletionLineIndex: before[0].deletionLineIndex, }, ]); - expect(diff.hunks[0].additionLines).toBe(0); - // The context-only hunk still renders every one of its rows. expect(countRenderedRows(diff)).toBe(rowsBefore); + expectPairingParity(diff); }); - test('an insert inside a region grows it and shifts later regions', () => { + test('synthesizes separate regions for separate canonical blocks in one gap', () => { const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); - diff.additionLines.splice(3, 0, 'inserted\n'); + const before = oldBounds(diff); + diff.additionLines[10] = 'replaced a\n'; + diff.additionLines[13] = 'replaced b\n'; - const change = applySessionEditWindow(diff, { - start: 3, - prevEnd: 3, - nextEnd: 4, - }); + const change = rebuildSessionHunks(diff); - expect(change).toBeUndefined(); - expect(diff.hunks).toHaveLength(2); - expect(diff.hunks[0].additionCount).toBe(boundsBefore[0].additionCount + 1); - expect(diff.hunks[0].deletionCount).toBe(boundsBefore[0].deletionCount); - expect(diff.hunks[1].additionLineIndex).toBe( - boundsBefore[1].additionLineIndex + 1 - ); - expect(diff.hunks[1].deletionLineIndex).toBe( - boundsBefore[1].deletionLineIndex - ); - // Trailing unchanged context must stay symmetric between sides. - expect(() => - getTrailingContextRangeSize({ fileDiff: diff, errorPrefix: 'test' }) - ).not.toThrow(); + expect(change?.regions).toEqual([ + { firstIndex: 0, lastIndex: 0 }, + undefined, + undefined, + { firstIndex: 1, lastIndex: 1 }, + ]); + expect(diff.hunks).toHaveLength(4); + expect(diff.hunks[1].deletionLineIndex).toBe(10); + expect(diff.hunks[2].deletionLineIndex).toBe(13); + expect(oldBounds(diff)[0]).toEqual(before[0]); + expect(oldBounds(diff)[3]).toEqual(before[1]); + expectPairingParity(diff); }); - test('an edit spanning the gap merges both regions and absorbs paired gap lines', () => { + test('merges regions only when a canonical block crosses their gap', () => { const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); - diff.additionLines[5] = 'edited 6\n'; - diff.additionLines[20] = 'edited 21\n'; - - const change = applySessionEditWindow(diff, { - start: 5, - prevEnd: 21, - nextEnd: 21, - }); + const before = oldBounds(diff); + diff.additionLines.splice(5, 16, 'one bridge\n', 'two bridge\n'); - expect(change).toEqual({ - type: 'merge', - firstIndex: 0, - lastIndex: 1, - previousHunkCount: 2, - }); + const change = rebuildSessionHunks(diff); + + expect(change?.regions).toEqual([{ firstIndex: 0, lastIndex: 1 }]); expect(diff.hunks).toHaveLength(1); - const merged = diff.hunks[0]; - expect(merged.additionLineIndex).toBe(boundsBefore[0].additionLineIndex); - expect(merged.additionCount).toBe( - boundsBefore[1].additionLineIndex + - boundsBefore[1].additionCount - - boundsBefore[0].additionLineIndex - ); - // Gap lines are absorbed in paired correspondence, so the deletion side - // spans the same range. - expect(merged.deletionLineIndex).toBe(boundsBefore[0].deletionLineIndex); - expect(merged.deletionCount).toBe( - boundsBefore[1].deletionLineIndex + - boundsBefore[1].deletionCount - - boundsBefore[0].deletionLineIndex + expect(diff.hunks[0].deletionLineIndex).toBe(before[0][0]); + expect(diff.hunks[0].deletionLineIndex + diff.hunks[0].deletionCount).toBe( + before[1][0] + before[1][1] ); - // The previously collapsed gap now renders as context inside the region. - expect( - merged.hunkContent.some((content) => content.type === 'context') - ).toBe(true); + expectPairingParity(diff); }); - test('a pure insert into a gap synthesizes a region anchored on a context line', () => { - const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); - diff.additionLines.splice(12, 0, 'new a\n', 'new b\n'); + test('keeps full-parse pairing through repeated lines and sequential passes', () => { + const oldContents = [ + '
', + '
', + '
', + '', + '', + '', + '', + '', + '