diff --git a/apps/staged/src-tauri/src/diff_commands.rs b/apps/staged/src-tauri/src/diff_commands.rs index a0ee7b088..72948868d 100644 --- a/apps/staged/src-tauri/src/diff_commands.rs +++ b/apps/staged/src-tauri/src/diff_commands.rs @@ -548,6 +548,12 @@ pub(crate) fn get_diff_files_impl( &branch_id, sha, ) { + log::info!( + "get_diff_files: remote branch cache hit in {:?}: {} files (sha={})", + start.elapsed(), + cached.files.len(), + &sha[..7.min(sha.len())] + ); return Ok(DiffFilesResponse { commit_sha: sha.clone(), files: cached.files, @@ -564,6 +570,12 @@ pub(crate) fn get_diff_files_impl( &branch_id, sha, ) { + log::info!( + "get_diff_files: remote commit cache hit in {:?}: {} files (sha={})", + start.elapsed(), + cached.files.len(), + &sha[..7.min(sha.len())] + ); return Ok(DiffFilesResponse { commit_sha: sha.clone(), files: cached.files, @@ -572,8 +584,17 @@ pub(crate) fn get_diff_files_impl( } } + log::info!( + "get_diff_files: remote cache miss, populating cache (scope={scope}, branch_id={branch_id})" + ); + let t_populate = std::time::Instant::now(); let (index, _, commit_results) = ensure_cache_populated(&ctx, &store, &branch_id, &scope, commit_sha.as_deref())?; + log::info!( + "get_diff_files: ensure_cache_populated done in {:?} (total {:?})", + t_populate.elapsed(), + start.elapsed() + ); if scope == "commit" { let sha = commit_sha.ok_or("commit_sha required for commit scope")?; @@ -662,6 +683,10 @@ pub(crate) fn get_file_diff_impl( &commit_sha, &path, ) { + log::info!( + "get_file_diff: remote branch cache hit in {:?}: path={path}", + start.elapsed() + ); return Ok(file_diff); } } @@ -675,12 +700,23 @@ pub(crate) fn get_file_diff_impl( &commit_sha, &path, ) { + log::info!( + "get_file_diff: remote commit cache hit in {:?}: path={path}", + start.elapsed() + ); return Ok(file_diff); } } + log::info!("get_file_diff: remote cache miss, populating cache (scope={scope}, path={path})"); + let t_populate = std::time::Instant::now(); let (_, branch_file_diffs, commit_results) = ensure_cache_populated(&ctx, &store, &branch_id, &scope, Some(&commit_sha))?; + log::info!( + "get_file_diff: ensure_cache_populated done in {:?} (total {:?})", + t_populate.elapsed(), + start.elapsed() + ); if scope == "commit" { if let Some(diff) = commit_results diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 98fcb5651..ba2bc3d9d 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -39,6 +39,7 @@ import ImageViewerModal from '../timeline/ImageViewerModal.svelte'; import { countUserComments, shouldWarnBeforeDeletingReview } from '../timeline/reviewState'; import DiffModal from '../diff/DiffModal.svelte'; + import { markDiffOpenClick } from '../diff/diffViewerState.svelte'; import SessionModal from '../sessions/SessionModal.svelte'; import NewSessionModal from '../sessions/NewSessionModal.svelte'; import NoteModal from '../notes/NoteModal.svelte'; @@ -845,6 +846,7 @@ } function handleCommitClick(sha: string) { + markDiffOpenClick(); commitDiffSha = sha; } @@ -863,6 +865,7 @@ const cached = timelineReviewDetailsById[reviewId]; if (cached) { reviewDiffTarget = { commitSha: cached.commitSha, scope: cached.scope, reviewId }; + markDiffOpenClick(); showBranchDiff = true; return; } @@ -875,6 +878,7 @@ return; } reviewDiffTarget = { commitSha: review.commitSha, scope: review.scope, reviewId }; + markDiffOpenClick(); showBranchDiff = true; } catch (e) { console.error('Failed to open review:', e); @@ -1458,7 +1462,12 @@ : undefined} {forcePushingOrigin} rebaseBranchDisabledReason={branchCommandDisabledReason} - onViewWorktreeDiff={isLocal ? () => (showWorktreeDiff = true) : undefined} + onViewWorktreeDiff={isLocal + ? () => { + markDiffOpenClick(); + showWorktreeDiff = true; + } + : undefined} onCommitWorktreeChanges={() => sessionMgr.startOrQueueSession('commit', 'Commit uncommitted changes')} onDiscardWorktreeChanges={handleDiscardWorktreeChanges} @@ -1490,6 +1499,7 @@ size="sm" onclick={() => { reviewDiffTarget = null; + markDiffOpenClick(); showBranchDiff = true; }} class="text-xs" diff --git a/apps/staged/src/lib/features/diff/DiffModal.svelte b/apps/staged/src/lib/features/diff/DiffModal.svelte index 78406c643..7ab11b387 100644 --- a/apps/staged/src/lib/features/diff/DiffModal.svelte +++ b/apps/staged/src/lib/features/diff/DiffModal.svelte @@ -645,6 +645,28 @@ // ========================================================================== let currentDiff = $derived(diffViewer.getCurrentDiff()); + + // Render-tail timing: `computeLineDiff` is sub-millisecond, but the syntax + // highlighting, DOM construction, and layout/paint the DiffViewer performs + // afterwards are never measured. Once a file's diff is in the cache and + // selected, time how long it takes to actually hit the screen via a + // double-rAF (the second frame fires after the browser has painted). Guarded + // by path so unrelated reactive updates (comments, search) don't re-log. + let renderTimedPath: string | null = null; + $effect(() => { + const path = diffViewer.state.selectedFile; + const ready = currentDiff !== null && diffViewer.state.loadingFile === null; + if (!path || !ready || renderTimedPath === path) return; + renderTimedPath = path; + const t0 = performance.now(); + requestAnimationFrame(() => { + requestAnimationFrame(() => { + console.info( + `[diff] render painted in ${Math.round(performance.now() - t0)}ms: path=${path}` + ); + }); + }); + }); let diffViewerEmptyMessage = $derived( diffViewer.state.loading || diffViewer.state.loadingFile !== null ? 'Loading changes...' diff --git a/apps/staged/src/lib/features/diff/diffViewerState.svelte.ts b/apps/staged/src/lib/features/diff/diffViewerState.svelte.ts index 16df8640f..17a71a288 100644 --- a/apps/staged/src/lib/features/diff/diffViewerState.svelte.ts +++ b/apps/staged/src/lib/features/diff/diffViewerState.svelte.ts @@ -19,6 +19,19 @@ export interface DiffViewerState { error: string | null; } +/** + * Timestamp of the most recent diff-open trigger (e.g. a "Diff" button click), + * recorded by `markDiffOpenClick`. Lets `createDiffViewerState` log the + * click→open gap — the otherwise-invisible window spent mounting DiffModal and + * its statically-imported DiffViewer before any diff work begins. + */ +let lastOpenClickAt: number | null = null; + +/** Stamp the moment a diff-open was triggered, just before showing DiffModal. */ +export function markDiffOpenClick() { + lastOpenClickAt = performance.now(); +} + /** * Create a reactive diff viewer state instance, pre-bound to Staged's Tauri commands. */ @@ -38,20 +51,41 @@ export function createDiffViewerState(branchId: string, scope: DiffScope, commit let selectionGeneration = 0; let contextGeneration = 0; + const clickToOpen = + lastOpenClickAt !== null + ? ` clickToOpen=${Math.round(performance.now() - lastOpenClickAt)}ms` + : ''; + lastOpenClickAt = null; + console.info( + `[diff] open: branchId=${branchId} scope=${scope} commitSha=${commitSha ?? '(unresolved)'}${clickToOpen}` + ); + async function loadFiles(generation: number): Promise { state.loading = true; state.error = null; + const t0 = performance.now(); + console.info( + `[diff] loadFiles start: branchId=${state.branchId} scope=${state.scope} commitSha=${state.commitSha ?? '(unresolved)'}` + ); try { const response = await commands.getDiffFiles( state.branchId, state.commitSha ?? undefined, state.scope ); - if (generation !== contextGeneration) return; + if (generation !== contextGeneration) { + console.info( + `[diff] loadFiles stale (took ${Math.round(performance.now() - t0)}ms) — ignoring` + ); + return; + } state.commitSha = response.commitSha; state.files = response.files; + console.info( + `[diff] loadFiles done in ${Math.round(performance.now() - t0)}ms: files=${response.files.length} commitSha=${response.commitSha}` + ); if (state.files.length > 0) { await selectFile(sharedFileSummaryPath(state.files[0])); @@ -60,6 +94,9 @@ export function createDiffViewerState(branchId: string, scope: DiffScope, commit if (generation !== contextGeneration) return; state.error = e instanceof Error ? e.message : String(e); state.files = []; + console.warn( + `[diff] loadFiles failed in ${Math.round(performance.now() - t0)}ms: ${state.error}` + ); } finally { if (generation === contextGeneration) { state.loading = false; @@ -81,18 +118,29 @@ export function createDiffViewerState(branchId: string, scope: DiffScope, commit if (!state.commitSha) return null; const cached = state.diffCache.get(path); - if (cached) return cached; + if (cached) { + console.info(`[diff] loadFileDiff cache hit: path=${path}`); + return cached; + } state.loadingFile = path; + const t0 = performance.now(); + console.info(`[diff] loadFileDiff start: path=${path} scope=${state.scope}`); try { const diff = await commands.getFileDiff(state.branchId, state.commitSha, state.scope, path); const newCache = new Map(state.diffCache); newCache.set(path, diff); state.diffCache = newCache; + console.info( + `[diff] loadFileDiff done in ${Math.round(performance.now() - t0)}ms: path=${path}` + ); return diff; } catch (e) { - console.error(`Failed to load diff for ${path}:`, e); + console.error( + `[diff] loadFileDiff failed in ${Math.round(performance.now() - t0)}ms: path=${path}:`, + e + ); return null; } finally { state.loadingFile = null; @@ -109,6 +157,7 @@ export function createDiffViewerState(branchId: string, scope: DiffScope, commit newCommitSha?: string ): Promise { const generation = ++contextGeneration; + console.info(`[diff] switchContext: scope=${newScope} commitSha=${newCommitSha ?? '(none)'}`); state.scope = newScope; state.commitSha = newCommitSha ?? null; state.diffCache = new Map(); diff --git a/packages/diff-viewer/src/lib/components/DiffViewer.svelte b/packages/diff-viewer/src/lib/components/DiffViewer.svelte index 6284f3925..f895ae780 100644 --- a/packages/diff-viewer/src/lib/components/DiffViewer.svelte +++ b/packages/diff-viewer/src/lib/components/DiffViewer.svelte @@ -251,6 +251,7 @@ let lastHandledJumpLineToken = $state(null); let lastAutoScrolledFile: string | null = null; let lineCommentEditorRaf: number | null = null; + let lineSelectionToolbarRaf: number | null = null; // Markdown preview mode let markdownPreview = $state(false); @@ -430,7 +431,9 @@ $effect(() => { if (beforePane && beforeLines.length > 0) { const lineHeight = measureLineHeight(beforePane); - const contentWidth = measureContentWidth(beforePane); + const contentWidth = measureContentWidth(beforeLines, beforePane); + beforeLineHeight = lineHeight || 20; + beforeViewportHeight = beforePane.clientHeight; scrollController.setDimensions('before', { viewportHeight: beforePane.clientHeight, contentHeight: beforeLines.length * lineHeight, @@ -444,7 +447,9 @@ $effect(() => { if (afterPane && afterLines.length > 0) { const lineHeight = measureLineHeight(afterPane); - const contentWidth = measureContentWidth(afterPane); + const contentWidth = measureContentWidth(afterLines, afterPane); + afterLineHeight = lineHeight || 20; + afterViewportHeight = afterPane.clientHeight; scrollController.setDimensions('after', { viewportHeight: afterPane.clientHeight, contentHeight: afterLines.length * lineHeight, @@ -473,16 +478,76 @@ let beforeContentHeight = $state(0); let afterContentHeight = $state(0); + // Reactive line/viewport metrics that drive the rendered window (Phase 2 + // virtualization). Kept in sync wherever dimensions are measured below so the + // window deriveds recompute on resize and font changes, not just on scroll. + let beforeLineHeight = $state(20); + let afterLineHeight = $state(20); + let beforeViewportHeight = $state(0); + let afterViewportHeight = $state(0); + // Content width needs to be measured after DOM renders, using state + effect let beforeContentWidth = $state(0); let afterContentWidth = $state(0); + // ========================================================================== + // Rendered window (Phase 2 virtualization) + // ========================================================================== + + /** + * Rows rendered above/below the viewport. Generous enough to cover the + * comment editor's vertical extent (it can render above a multi-line range), + * so anchor lookups near a viewport edge still resolve to a mounted row. + */ + const OVERSCAN = 40; + + /** + * Compute the absolute index range [start, end) to render for a pane given + * its scroll offset and metrics. Returns absolute indices so every existing + * `i`-consumer (highlighting, boundaries, class lookups) stays unchanged. + */ + function computeWindow( + total: number, + lineHeight: number, + scrollY: number, + viewportHeight: number + ): { start: number; indices: number[] } { + const lh = lineHeight || 20; + const firstVisible = Math.max(0, Math.floor(scrollY / lh)); + const visibleCount = Math.ceil((viewportHeight || 0) / lh); + const start = Math.max(0, firstVisible - OVERSCAN); + const end = Math.min(total, firstVisible + visibleCount + OVERSCAN); + const indices: number[] = []; + for (let i = start; i < end; i++) indices.push(i); + return { start, indices }; + } + + let beforeWindow = $derived( + computeWindow( + beforeLines.length, + beforeLineHeight, + scrollController.beforeScrollY, + beforeViewportHeight + ) + ); + + let afterWindow = $derived( + computeWindow( + afterLines.length, + afterLineHeight, + scrollController.afterScrollY, + afterViewportHeight + ) + ); + function updateContentWidths() { requestAnimationFrame(() => { if (beforePane) { const lh = measureLineHeight(beforePane) || 20; + beforeLineHeight = lh; + beforeViewportHeight = beforePane.clientHeight; beforeContentHeight = beforeLines.length * lh; - beforeContentWidth = measureContentWidth(beforePane); + beforeContentWidth = measureContentWidth(beforeLines, beforePane); scrollController.setDimensions('before', { viewportHeight: beforePane.clientHeight, contentHeight: beforeContentHeight, @@ -493,8 +558,10 @@ } if (afterPane) { const lh = measureLineHeight(afterPane) || 20; + afterLineHeight = lh; + afterViewportHeight = afterPane.clientHeight; afterContentHeight = afterLines.length * lh; - afterContentWidth = measureContentWidth(afterPane); + afterContentWidth = measureContentWidth(afterLines, afterPane); scrollController.setDimensions('after', { viewportHeight: afterPane.clientHeight, contentHeight: afterContentHeight, @@ -1185,6 +1252,41 @@ // Range hover handling // ========================================================================== + // Resolve a line element by its absolute index rather than by NodeList + // position. Positional lookups (`querySelectorAll('.line')[n]`) only equal the + // absolute index while every line is in the DOM; identity lookups stay correct + // once the body is windowed and only a slice of lines is rendered. + function lineAt(pane: HTMLElement, index: number): HTMLElement | null { + return pane.querySelector(`.line[data-line-index="${index}"]`); + } + + // Cap on the number of frames a deferred measure waits for the windowed body + // to render the rows it needs. A scroll typically renders within one frame; + // the budget absorbs a slow render pass without spinning indefinitely. + const WINDOW_RENDER_MAX_FRAMES = 5; + + // Phase 3: scroll → window-render → measure sequencing. Setting a pane's scroll + // offset recomputes its window and Svelte renders the new rows, but only on a + // later frame — so an imperative measure on the *next* frame can still find the + // anchor row unmounted, leaving the editor/toolbar silently unpositioned. Retry + // `measure` across a bounded number of frames until it reports success: it + // returns true to stop (positioned, or nothing to do) and false to retry. + // `trackRaf` lets the caller hold the pending frame id so a newer request can + // cancel an in-flight wait. + function measureAfterWindowRender( + measure: () => boolean, + trackRaf: (raf: number | null) => void + ) { + let frames = 0; + const tick = () => { + trackRaf(null); + if (measure() || frames >= WINDOW_RENDER_MAX_FRAMES) return; + frames++; + trackRaf(requestAnimationFrame(tick)); + }; + trackRaf(requestAnimationFrame(tick)); + } + function updateToolbarPosition() { if (hoveredRangeIndex === null || !afterPane || !diffViewerEl) { rangeToolbarStyle = null; @@ -1198,7 +1300,7 @@ } const lineIndex = alignmentData.alignment.after.start; - const lineEl = afterPane.querySelectorAll('.line')[lineIndex] as HTMLElement | null; + const lineEl = lineAt(afterPane, lineIndex); if (!lineEl) { rangeToolbarStyle = null; @@ -1258,18 +1360,63 @@ lineSelection = { pane: 'after', anchorLine: start, focusLine: end }; commentingOnLines = { pane: 'after', start, end }; - lineCommentPositionPreference = decideLineCommentPosition(); editingCommentId = comment.id; activeLineComment = comment; lineCommentReadOnly = comment.author === 'agent'; - // scrollToRow updates pane transforms; defer editor positioning until next frame + // scrollToRow updates pane transforms, but the windowed body only renders the + // target rows on a later frame — wait for the anchor to mount before deciding + // the editor's side and positioning it. + scheduleLineCommentEditorPositioning(); + } + + // Resolve the comment editor's side and screen position once the anchor rows are + // mounted. Returns true to stop the mount-then-measure retry (positioned, or + // nothing to position), false to retry on a later frame. + function positionLineCommentEditor(): boolean { + if (!commentingOnLines) return true; + + const pane = commentingOnLines.pane === 'before' ? beforePane : afterPane; + if (!pane) return true; + + const firstEl = lineAt(pane, commentingOnLines.start); + const lastEl = lineAt(pane, commentingOnLines.end); + + // Both ends mounted: full space-based decision (short ranges). + if (firstEl && lastEl) { + lineCommentPositionPreference = decideLineCommentPosition(); + updateLineCommentEditorPosition(); + return true; + } + + // Range taller than the window — both ends can't co-mount in one frame, so + // decideLineCommentPosition (which measures both) can never run. Anchor to + // whichever end is rendered. Callers scroll to `start` first, so it lands in + // the window; position the editor below it (there's space below after the + // scroll). Pass the resolved row explicitly so the anchor doesn't have to be + // re-derived from the preference against an unmounted row. + if (firstEl) { + lineCommentPositionPreference = 'below'; + updateLineCommentEditorPosition(firstEl); + return true; + } + if (lastEl) { + lineCommentPositionPreference = 'above'; + updateLineCommentEditorPosition(lastEl); + return true; + } + + // Neither end mounted yet: keep retrying for the window render. + return false; + } + + function scheduleLineCommentEditorPositioning() { if (lineCommentEditorRaf !== null) { cancelAnimationFrame(lineCommentEditorRaf); - } - lineCommentEditorRaf = requestAnimationFrame(() => { lineCommentEditorRaf = null; - updateLineCommentEditorPosition(); + } + measureAfterWindowRender(positionLineCommentEditor, (raf) => { + lineCommentEditorRaf = raf; }); } @@ -1298,11 +1445,10 @@ lineSelection = { pane: 'after', anchorLine: start, focusLine: end }; commentingOnLines = { pane: 'after', start, end }; - lineCommentPositionPreference = decideLineCommentPosition(); editingCommentId = commentId; activeLineComment = comment; lineCommentReadOnly = false; - updateLineCommentEditorPosition(); + scheduleLineCommentEditorPositioning(); } // Jump to a comment requested by the sidebar comments list. @@ -1347,15 +1493,13 @@ const editorHeight = 120; const lastLineIndex = Math.max(alignment.after.start, alignment.after.end - 1); - const lastLineEl = afterPane.querySelectorAll('.line')[lastLineIndex] as HTMLElement | null; + const lastLineEl = lineAt(afterPane, lastLineIndex); if (!lastLineEl) return 'below'; const lastLineRect = lastLineEl.getBoundingClientRect(); const spaceBelow = paneRect.bottom - lastLineRect.bottom; - const firstLineEl = afterPane.querySelectorAll('.line')[ - alignment.after.start - ] as HTMLElement | null; + const firstLineEl = lineAt(afterPane, alignment.after.start); if (!firstLineEl) return 'below'; const firstLineRect = firstLineEl.getBoundingClientRect(); @@ -1384,15 +1528,13 @@ if (commentPositionPreference === 'below') { const lastLineIndex = Math.max(alignment.after.start, alignment.after.end - 1); - anchorLineEl = afterPane.querySelectorAll('.line')[lastLineIndex] as HTMLElement | null; + anchorLineEl = lineAt(afterPane, lastLineIndex); if (!anchorLineEl) { commentEditorStyle = null; return; } } else { - anchorLineEl = afterPane.querySelectorAll('.line')[ - alignment.after.start - ] as HTMLElement | null; + anchorLineEl = lineAt(afterPane, alignment.after.start); if (!anchorLineEl) { commentEditorStyle = null; return; @@ -1458,18 +1600,27 @@ function handleSelectionDragMove(event: MouseEvent) { if (!isSelecting || !lineSelection) return; - const pane = lineSelection.pane === 'before' ? beforePane : afterPane; + const side = lineSelection.pane; + const pane = side === 'before' ? beforePane : afterPane; if (!pane) return; - const lineElements = pane.querySelectorAll('.line'); - for (let i = 0; i < lineElements.length; i++) { - const rect = lineElements[i].getBoundingClientRect(); - if (event.clientY >= rect.top && event.clientY < rect.bottom) { - if (lineSelection.focusLine !== i) { - lineSelection = { ...lineSelection, focusLine: i }; - } - break; - } + // Map the cursor to an absolute line index arithmetically rather than + // hit-testing rendered `.line` rects: the wrapper is transform-translated by + // -scrollY over a uniform lineHeight, and once the body is windowed the rows + // under the cursor may not be mounted. + const lineHeight = scrollController.getDimensions(side).lineHeight || 20; + const scrollY = + side === 'before' ? scrollController.beforeScrollY : scrollController.afterScrollY; + const total = (side === 'before' ? beforeLines.length : afterLines.length) - 1; + if (total < 0) return; + + const paneTop = pane.getBoundingClientRect().top; + const focusLine = Math.min( + Math.max(Math.floor((event.clientY - paneTop + scrollY) / lineHeight), 0), + total + ); + if (lineSelection.focusLine !== focusLine) { + lineSelection = { ...lineSelection, focusLine }; } } @@ -1481,12 +1632,28 @@ document.removeEventListener('mousemove', handleSelectionDragMove); if (lineSelection) { - requestAnimationFrame(() => { - updateLineSelectionToolbar(true); - }); + scheduleLineSelectionToolbar(true); } } + // Position the line-selection toolbar once the selection's anchor row is + // mounted. A drag that ends after auto-scrolling can leave the anchor outside + // the freshly rendered window for a frame, so wait for it like the editor. + function scheduleLineSelectionToolbar(recalculateLeft = false) { + measureAfterWindowRender( + () => { + if (!selectedLineRange) return true; + const pane = selectedLineRange.pane === 'before' ? beforePane : afterPane; + if (pane && !lineAt(pane, selectedLineRange.start)) return false; + updateLineSelectionToolbar(recalculateLeft); + return true; + }, + (raf) => { + lineSelectionToolbarRaf = raf; + } + ); + } + function clearLineSelection() { lineSelection = null; isSelecting = false; @@ -1521,8 +1688,7 @@ return; } - const lines = pane.querySelectorAll('.line'); - const lineEl = lines[selectedLineRange.start] as HTMLElement | null; + const lineEl = lineAt(pane, selectedLineRange.start); if (!lineEl) { lineSelectionToolbarStyle = null; lineSelectionToolbarLeft = null; @@ -1565,9 +1731,8 @@ const pane = commentingOnLines.pane === 'before' ? beforePane : afterPane; if (!pane) return 'below'; - const lines = pane.querySelectorAll('.line'); - const firstLineEl = lines[commentingOnLines.start] as HTMLElement | null; - const lastLineEl = lines[commentingOnLines.end] as HTMLElement | null; + const firstLineEl = lineAt(pane, commentingOnLines.start); + const lastLineEl = lineAt(pane, commentingOnLines.end); if (!firstLineEl || !lastLineEl) return 'below'; const paneRect = pane.getBoundingClientRect(); @@ -1581,7 +1746,7 @@ return decideCommentPositionBySpace(spaceBelow, spaceAbove, editorHeight); } - function updateLineCommentEditorPosition() { + function updateLineCommentEditorPosition(anchorOverride?: HTMLElement) { if (!commentingOnLines || !diffViewerEl) { lineCommentEditorStyle = null; return; @@ -1598,14 +1763,18 @@ const editorHeight = 120; let anchorLineEl: HTMLElement | null; - if (lineCommentPositionPreference === 'below') { - anchorLineEl = pane.querySelectorAll('.line')[commentingOnLines.end] as HTMLElement | null; + if (anchorOverride) { + // Tall-range path: the caller resolved the only mounted end; use it directly + // rather than re-deriving from the preference against an unmounted row. + anchorLineEl = anchorOverride; + } else if (lineCommentPositionPreference === 'below') { + anchorLineEl = lineAt(pane, commentingOnLines.end); if (!anchorLineEl) { lineCommentEditorStyle = null; return; } } else { - anchorLineEl = pane.querySelectorAll('.line')[commentingOnLines.start] as HTMLElement | null; + anchorLineEl = lineAt(pane, commentingOnLines.start); if (!anchorLineEl) { lineCommentEditorStyle = null; return; @@ -1733,19 +1902,15 @@ if (selectedLineRange) { event.preventDefault(); - const pane = selectedLineRange.pane === 'before' ? beforePane : afterPane; - if (!pane) return; - + // Reconstruct from the source-line array by index rather than the DOM: + // with the body windowed, rows outside the rendered slice aren't mounted, + // so a DOM-based read would silently drop lines from a tall selection. + const sourceLines = selectedLineRange.pane === 'before' ? beforeLines : afterLines; const lines: string[] = []; - const lineElements = pane.querySelectorAll('.line'); for (let i = selectedLineRange.start; i <= selectedLineRange.end; i++) { - const lineEl = lineElements[i]; - if (lineEl) { - const contentEl = lineEl.querySelector('.line-content'); - if (contentEl) { - lines.push(contentEl.textContent || ''); - } + if (i >= 0 && i < sourceLines.length) { + lines.push(sourceLines[i]); } } @@ -1829,6 +1994,10 @@ cancelAnimationFrame(lineCommentEditorRaf); lineCommentEditorRaf = null; } + if (lineSelectionToolbarRaf !== null) { + cancelAnimationFrame(lineSelectionToolbarRaf); + lineSelectionToolbarRaf = null; + } if (connectorRenderer) { connectorRenderer.destroy(); connectorRenderer = null; @@ -1946,7 +2115,11 @@ class="lines-wrapper" style="transform: translate(-{scrollController.beforeScrollX}px, -{scrollController.beforeScrollY}px)" > - {#each beforeLines as line, i} +
+ {#each beforeWindow.indices as i (i)} {@const boundary = showRangeMarkers ? getLineBoundary(activeAlignments, 'before', i) : { isStart: false, isEnd: false }} @@ -1957,6 +2130,7 @@
- {#each beforeLines as line, i} -
+
+ {#each beforeWindow.indices as i (i)} +
{#each getHighlightedTokens(i, 'before') as segment} - {#each afterLines as line, i} +
+ {#each afterWindow.indices as i (i)} {@const boundary = showRangeMarkers ? getLineBoundary(activeAlignments, 'after', i) : { isStart: false, isEnd: false }} @@ -2133,6 +2315,7 @@
- {#each afterLines as line, i} +
+ {#each afterWindow.indices as i (i)} {@const isSelected = isLineSelected('after', i)}
handleLineMouseDown('after', i, e)} > @@ -2760,6 +2948,7 @@ position: relative; } + /* Top spacer reserving the height of unrendered rows above the window. */ .line-content { flex: 1; padding: 0 12px; diff --git a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts index 05f50cf8a..17ea262ad 100644 --- a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts +++ b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts @@ -205,10 +205,68 @@ export function measureLineHeight(pane: HTMLElement | null): number { return firstLine ? firstLine.getBoundingClientRect().height : 20; } -export function measureContentWidth(pane: HTMLElement | null): number { +/** Horizontal padding on `.line-content` (`padding: 0 12px`), in pixels. */ +const LINE_CONTENT_PADDING_X = 24; + +/** + * Display-column count of a line, expanding tabs to the next 8-column tab stop + * (the CSS sets no `tab-size`, so the default 8 applies). Iterates by code + * point so astral characters count as one. Under a monospace font this is a + * faithful proxy for which line is widest in pixels. + */ +function displayColumns(line: string): number { + let columns = 0; + for (const ch of line) { + if (ch === '\t') columns += 8 - (columns % 8); + else columns += 1; + } + return columns; +} + +/** + * Width of the widest line in pixels, measured from the source text rather than + * the rendered DOM. + * + * The diff body is virtualized — only a window of lines is mounted — so reading + * `.lines-wrapper`'s `scrollWidth` only sees the widest *rendered* line and + * under-clamps horizontal scrolling when the file's longest line is offscreen. + * Instead, pick the widest line by display-column count and measure just that + * one line in an offscreen probe span that inherits `.line-content`'s font and + * `white-space: pre`. One node, so virtualization is preserved, and the result + * reflects the whole file regardless of scroll position. + * + * Returns at least `pane.clientWidth` to keep the `.lines-wrapper` + * `min-width: 100%` floor. + */ +export function measureContentWidth(lines: string[], pane: HTMLElement | null): number { if (!pane) return 0; - const linesWrapper = pane.querySelector('.lines-wrapper') as HTMLElement | null; - return linesWrapper ? linesWrapper.scrollWidth : 0; + if (lines.length === 0) return 0; + + let widest = ''; + let maxColumns = -1; + for (const line of lines) { + const columns = displayColumns(line); + if (columns > maxColumns) { + maxColumns = columns; + widest = line; + } + } + + // Probe inside the pane so it inherits the live computed font (font-family, + // size, and any --code-font-size override), matching the rendered lines. + const probe = document.createElement('span'); + probe.textContent = widest; + probe.style.position = 'absolute'; + probe.style.top = '0'; + probe.style.left = '0'; + probe.style.visibility = 'hidden'; + probe.style.whiteSpace = 'pre'; + probe.style.pointerEvents = 'none'; + pane.appendChild(probe); + const textWidth = probe.getBoundingClientRect().width; + pane.removeChild(probe); + + return Math.max(Math.ceil(textWidth) + LINE_CONTENT_PADDING_X, pane.clientWidth); } export function getTokensForLine(tokens: Token[][], index: number): Token[] {