From a12d266a301b8066894989edc98c9990da91d6c4 Mon Sep 17 00:00:00 2001 From: wonder Date: Fri, 19 Jun 2026 20:29:29 +0800 Subject: [PATCH] =?UTF-8?q?perf:=20=E5=A2=9E=E9=87=8F=20diff=20=E6=B8=B2?= =?UTF-8?q?=E6=9F=93=EF=BC=8C=E6=B6=88=E9=99=A4=E4=B8=BB=E7=BA=BF=E7=A8=8B?= =?UTF-8?q?=E9=98=BB=E5=A1=9E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- static/js/diff-viewer.js | 420 ++++++++++++++++++++++-------------- templates/pages/review.html | 14 +- 2 files changed, 266 insertions(+), 168 deletions(-) diff --git a/static/js/diff-viewer.js b/static/js/diff-viewer.js index 8633c7b..79016bd 100644 --- a/static/js/diff-viewer.js +++ b/static/js/diff-viewer.js @@ -1,4 +1,5 @@ // Diff Viewer using diff2html with file tree sidebar +// Optimized: incremental rendering, lazy indexing, collapsed-by-default const DiffViewer = { init(containerId, options = {}) { this.container = document.getElementById(containerId); @@ -8,9 +9,12 @@ const DiffViewer = { this.files = []; // parsed file info for tree this.observer = null; // scroll observer for highlight sync this._highlightObserver = null; // lazy hljs observer - this._fileIndex = null; // filename → { wrapper, rows: Map } - this._lastActiveFile = null; // last highlighted tree item - this._rafPending = false; // rAF throttle flag for scroll spy + this._fileIndex = null; // filename → { wrapper, rows: Map, built: bool } + this._lastActiveFile = null; + this._rafPending = false; + this._renderQueue = []; // per-file chunks waiting to be rendered + this._rendering = false; + this._onRenderComplete = null; // callback when incremental render finishes if (!this.container) { console.error('Diff container not found:', containerId); @@ -55,12 +59,33 @@ const DiffViewer = { additions, deletions, changeType, + _chunk: chunk, // keep chunk for incremental rendering }); } return files; }, + // Split diff string into per-file chunks (returns array of strings) + _splitDiffIntoChunks(diffString) { + if (!diffString) return []; + const fileRegex = /^diff --git a\/.*? b\/.*?$/gm; + const positions = []; + let match; + while ((match = fileRegex.exec(diffString)) !== null) { + positions.push(match.index); + } + if (positions.length === 0) return diffString ? [diffString] : []; + + const chunks = []; + for (let i = 0; i < positions.length; i++) { + const start = positions[i]; + const end = i + 1 < positions.length ? positions[i + 1] : diffString.length; + chunks.push(diffString.substring(start, end)); + } + return chunks; + }, + // Build tree structure from flat file list _buildTree(files) { const root = { name: '', path: '', children: [], isDir: true }; @@ -137,26 +162,19 @@ const DiffViewer = { let html = ''; if (node.isDir) { - // Only render directory label if not root if (depth > 0) { html += `
▶ ${node.name}/
`; } - // Render children const childDepth = depth > 0 ? depth + 1 : depth; if (node.children) { node.children.forEach(child => { - if (child.isDir) { - html += this._renderTreeNode(child, childDepth); - } else { - html += this._renderTreeNode(child, childDepth); - } + html += this._renderTreeNode(child, childDepth); }); } } else { - // File node const icon = this._getFileIcon(node.changeType); html += `
@@ -181,64 +199,65 @@ const DiffViewer = { } }, - // Render diff with file tree sidebar + // ── Incremental rendering pipeline ────────────────────────────── + + // Render diff with file tree sidebar — incremental, non-blocking renderDiff(diffString, options = {}) { if (!this.container) return; - // Store raw diff for view mode switching - this.rawDiff = diffString; + // Cancel any in-progress incremental render + this._cancelRender(); - // Parse files for tree + this.rawDiff = diffString; + this._onRenderComplete = options.onComplete || null; + + // Parse files for tree (fast — just regex + string ops) this.files = this._parseDiffFiles(diffString); const tree = this._buildTree(this.files); - const config = { - drawFileList: false, // We use our own file tree - fileListToggle: false, - fileContentToggle: true, - matching: options.matching || 'lines', - outputFormat: options.outputFormat || this.currentMode === 'unified' ? 'line-by-line' : 'side-by-side', - synchronisedScroll: true, - highlight: true, - renderNothingWhenEmpty: false, - }; + // Split diff into per-file chunks for incremental rendering + this._chunks = this._splitDiffIntoChunks(diffString); + this._chunkIndex = 0; + this._renderedCount = 0; - this.currentMode = config.outputFormat === 'line-by-line' ? 'unified' : 'side-by-side'; + // Determine initial mode + const outputFormat = options.outputFormat || + (this.currentMode === 'unified' ? 'line-by-line' : 'side-by-side'); + this.currentMode = outputFormat === 'line-by-line' ? 'unified' : 'side-by-side'; - // Build layout: sidebar + diff area - const diffHtml = Diff2Html.html(diffString, config); + // Build layout shell: sidebar + empty diff area const treeHtml = this._renderFileTree(tree); - this.container.innerHTML = `
${treeHtml}
- ${diffHtml} +
+
`; - // Add data-filename attributes to file wrappers for scroll sync - this._annotateFileSections(); + this._diffContainer = this.container.querySelector('#diff-file-container'); + this._progressEl = this.container.querySelector('#diff-render-progress'); - // Build index for fast suggestion insertion - this._buildFileIndex(); - - // Bind file tree click events + // Bind file tree events (fast — just the sidebar) this._bindFileTreeEvents(); - // Set up scroll spy - this._setupScrollSpy(); + // Reset lazy index + this._fileIndex = null; - // Lazy syntax highlighting (only when code blocks scroll into view) - this._setupLazyHighlight(); + // Start incremental render: first batch paints immediately + this._renderBatch(outputFormat, 0); }, - // Render per-file diffs + // Render per-file diffs — incremental, non-blocking renderFiles(files, options = {}) { if (!this.container) return; + this._cancelRender(); + this._onRenderComplete = options.onComplete || null; + if (!files || files.length === 0) { this.container.innerHTML = '
No changes
'; return; @@ -258,99 +277,194 @@ const DiffViewer = { additions, deletions, changeType: 'modified', + _patch: f.patch, }; }); + // Build combined diff for rawDiff storage const combinedDiff = files.map(f => f.patch).join('\n'); this.rawDiff = combinedDiff; + // Split into per-file chunks + this._chunks = this.files.map(f => f._patch).filter(Boolean); + this._chunkIndex = 0; + this._renderedCount = 0; + const tree = this._buildTree(this.files); - const config = { - drawFileList: false, - fileListToggle: false, - fileContentToggle: true, - matching: options.matching || 'lines', - outputFormat: options.outputFormat || (this.currentMode === 'unified' ? 'line-by-line' : 'side-by-side'), - synchronisedScroll: true, - highlight: true, - renderNothingWhenEmpty: false, - }; - const diffHtml = Diff2Html.html(combinedDiff, config); + const outputFormat = options.outputFormat || + (this.currentMode === 'unified' ? 'line-by-line' : 'side-by-side'); + this.currentMode = outputFormat === 'line-by-line' ? 'unified' : 'side-by-side'; + const treeHtml = this._renderFileTree(tree); - this.container.innerHTML = `
${treeHtml}
- ${diffHtml} +
+
`; - this._annotateFileSections(); - this._buildFileIndex(); + this._diffContainer = this.container.querySelector('#diff-file-container'); + this._progressEl = this.container.querySelector('#diff-render-progress'); + this._bindFileTreeEvents(); - this._setupScrollSpy(); - this._setupLazyHighlight(); + this._fileIndex = null; + + this._renderBatch(outputFormat, 0); }, - // Annotate diff2html file wrappers with data-filename - _annotateFileSections() { - const fileWrappers = this.container.querySelectorAll('.d2h-file-wrapper'); - fileWrappers.forEach(wrapper => { + // Cancel in-progress incremental render + _cancelRender() { + if (this._renderRaf) { + cancelAnimationFrame(this._renderRaf); + this._renderRaf = null; + } + this._rendering = false; + this._renderQueue = []; + }, + + // Render a batch of files per animation frame + // Each frame renders up to FILES_PER_BATCH files, then yields + _renderBatch(outputFormat, startIndex) { + const FILES_PER_BATCH = 5; // render 5 files per frame + const chunks = this._chunks; + if (!chunks || startIndex >= chunks.length) { + // All done + this._rendering = false; + if (this._progressEl) this._progressEl.textContent = ''; + // Finalize: set up scroll spy after all content is rendered + this._setupScrollSpy(); + this._setupLazyHighlight(); + // Fire completion callback + if (this._onRenderComplete) { + const cb = this._onRenderComplete; + this._onRenderComplete = null; + cb(); + } + return; + } + + this._rendering = true; + const endIndex = Math.min(startIndex + FILES_PER_BATCH, chunks.length); + const batch = chunks.slice(startIndex, endIndex); + + // Generate HTML for this batch + const config = { + drawFileList: false, + fileListToggle: false, + fileContentToggle: true, + matching: 'lines', + outputFormat: outputFormat === 'unified' ? 'line-by-line' : 'side-by-side', + synchronisedScroll: true, + highlight: false, // we handle hljs lazily + renderNothingWhenEmpty: false, + }; + + // Render each file chunk individually (much smaller strings) + const fragment = document.createDocumentFragment(); + for (const chunk of batch) { + const html = Diff2Html.html(chunk, config); + const wrapper = document.createElement('div'); + wrapper.innerHTML = html; + // Diff2Html wraps each file in .d2h-file-wrapper, move it out + const fileWrapper = wrapper.querySelector('.d2h-file-wrapper'); + if (fileWrapper) { + fragment.appendChild(fileWrapper); + } + } + + // Append to DOM in one operation (much smaller than full render) + this._diffContainer.appendChild(fragment); + + // Update progress + this._renderedCount = endIndex; + if (this._progressEl && endIndex < chunks.length) { + this._progressEl.textContent = `渲染中 ${endIndex}/${chunks.length} 文件...`; + } + + // Annotate newly added file wrappers + this._annotateNewFileWrappers(); + + // Schedule next batch + this._renderRaf = requestAnimationFrame(() => { + this._renderBatch(outputFormat, endIndex); + }); + }, + + // Annotate only newly added file wrappers (not all of them) + _annotateNewFileWrappers() { + const fileWrappers = this._diffContainer.querySelectorAll('.d2h-file-wrapper'); + // Only annotate wrappers that don't have data-filename yet + for (const wrapper of fileWrappers) { + if (wrapper.hasAttribute('data-filename')) continue; const header = wrapper.querySelector('.d2h-file-name'); if (header) { const name = header.textContent.trim(); - // Extract just the filename from the header (diff2html adds a prefix) const cleanName = name.replace(/^\s*(Modified|Added|Deleted|Rename)\s*/i, '').trim(); wrapper.setAttribute('data-filename', cleanName); } - }); - }, - - // Build index: filename → { wrapper, rows: Map } for O(1) suggestion lookup - _buildFileIndex() { - this._fileIndex = new Map(); - const diffContent = this.container.querySelector('#diff-content'); - if (!diffContent) return; - - const fileWrappers = diffContent.querySelectorAll('.d2h-file-wrapper'); - fileWrappers.forEach(wrapper => { - const filename = wrapper.getAttribute('data-filename'); - if (!filename) return; - - const rows = new Map(); - const table = wrapper.querySelector('.d2h-diff-table'); - if (table) { - table.querySelectorAll('tr').forEach(row => { - // Index by left (old) line number - const leftCell = row.querySelector('.d2h-code-linenumber .d2h-code-side-linenumber'); - if (leftCell) { - const num = parseInt(leftCell.textContent.trim()); - if (num) rows.set('left-' + num, row); - } - // Index by right (new) line number - const rightCell = row.querySelector('.d2h-code-linenumber:not(.d2h-code-side-linenumber)'); - if (rightCell) { - const num = parseInt(rightCell.textContent.trim()); - if (num) rows.set('right-' + num, row); - } - }); + // Collapse file content by default for faster initial render + const content = wrapper.querySelector('.d2h-file-diff'); + if (content) { + content.style.display = 'none'; + wrapper.classList.add('d2h-collapsed'); } - - this._fileIndex.set(filename, { wrapper, rows }); - }); + } }, - // Lazy syntax highlighting: only highlight code blocks when they scroll into view + // ── Lazy file index ───────────────────────────────────────────── + + // Build index for a single file on demand (called by insertSuggestion) + _ensureFileIndexed(filename) { + if (!this._fileIndex) this._fileIndex = new Map(); + + // Check if already indexed + if (this._fileIndex.has(filename)) return this._fileIndex.get(filename); + + // Find the wrapper + const fileWrappers = this._diffContainer + ? this._diffContainer.querySelectorAll('.d2h-file-wrapper') + : this.container.querySelectorAll('.d2h-file-wrapper'); + + for (const wrapper of fileWrappers) { + const wrapperName = wrapper.getAttribute('data-filename'); + if (wrapperName && (wrapperName === filename || wrapperName.endsWith('/' + filename) || filename.endsWith('/' + wrapperName))) { + const entry = { wrapper, rows: new Map() }; + const table = wrapper.querySelector('.d2h-diff-table'); + if (table) { + table.querySelectorAll('tr').forEach(row => { + const leftCell = row.querySelector('.d2h-code-linenumber .d2h-code-side-linenumber'); + if (leftCell) { + const num = parseInt(leftCell.textContent.trim()); + if (num) entry.rows.set('left-' + num, row); + } + const rightCell = row.querySelector('.d2h-code-linenumber:not(.d2h-code-side-linenumber)'); + if (rightCell) { + const num = parseInt(rightCell.textContent.trim()); + if (num) entry.rows.set('right-' + num, row); + } + }); + } + this._fileIndex.set(filename, entry); + return entry; + } + } + + return null; + }, + + // ── Lazy syntax highlighting ──────────────────────────────────── + _setupLazyHighlight() { if (this._highlightObserver) { this._highlightObserver.disconnect(); } - const codeBlocks = this.container.querySelectorAll('pre code'); + const codeBlocks = (this._diffContainer || this.container).querySelectorAll('pre code'); if (codeBlocks.length === 0 || !window.hljs) return; this._highlightObserver = new IntersectionObserver((entries) => { @@ -364,14 +478,15 @@ const DiffViewer = { } }); }, { - rootMargin: '200px', // start highlighting 200px before visible + rootMargin: '200px', threshold: 0, }); codeBlocks.forEach(block => this._highlightObserver.observe(block)); }, - // Bind click events on file tree items + // ── File tree events ──────────────────────────────────────────── + _bindFileTreeEvents() { const treeItems = this.container.querySelectorAll('.file-tree-file'); treeItems.forEach(item => { @@ -379,8 +494,9 @@ const DiffViewer = { const filename = item.getAttribute('data-filename'); this.scrollToFile(filename); - // Highlight active file in tree - this.container.querySelectorAll('.file-tree-file').forEach(fi => fi.classList.remove('active')); + // Toggle active — only remove from previous, not full scan + const prev = this.container.querySelector('.file-tree-file.active'); + if (prev) prev.classList.remove('active'); item.classList.add('active'); }); }); @@ -395,10 +511,8 @@ const DiffViewer = { toggle.textContent = isExpanded ? '▶' : '▼'; - // Toggle children visibility let sibling = dir.nextElementSibling; while (sibling) { - // Check if this sibling is still a child (starts with same path or is a file at same level) const sibPath = sibling.getAttribute('data-path') || sibling.getAttribute('data-filename') || ''; if (!sibPath.startsWith(path + '/') && sibling.classList.contains('file-tree-dir')) { break; @@ -410,7 +524,8 @@ const DiffViewer = { }); }, - // Scroll to a specific file section in the diff + // ── Scroll to file ────────────────────────────────────────────── + scrollToFile(filename) { const diffContent = this.container.querySelector('#diff-content'); if (!diffContent) return; @@ -419,8 +534,13 @@ const DiffViewer = { for (const wrapper of fileWrappers) { const wrapperName = wrapper.getAttribute('data-filename'); if (wrapperName && (wrapperName === filename || wrapperName.endsWith('/' + filename) || filename.endsWith('/' + wrapperName))) { + // Expand the file if collapsed + const content = wrapper.querySelector('.d2h-file-diff'); + if (content && content.style.display === 'none') { + content.style.display = 'block'; + wrapper.classList.remove('d2h-collapsed'); + } wrapper.scrollIntoView({ behavior: 'smooth', block: 'start' }); - // Brief highlight effect wrapper.style.outline = '2px solid #3b82f6'; setTimeout(() => { wrapper.style.outline = ''; }, 2000); return; @@ -428,12 +548,12 @@ const DiffViewer = { } }, - // Set up scroll spy to highlight current file in tree + // ── Scroll spy ────────────────────────────────────────────────── + _setupScrollSpy() { const diffContent = this.container.querySelector('#diff-content'); if (!diffContent) return; - // Clean up old observer if (this.observer) { this.observer.disconnect(); } @@ -445,19 +565,17 @@ const DiffViewer = { if (fileWrappers.length === 0) return; this.observer = new IntersectionObserver((entries) => { - // Throttle: batch all entries per frame, only process the most visible one if (this._rafPending) return; this._rafPending = true; requestAnimationFrame(() => { this._rafPending = false; - // Find the most recently intersecting entry let bestEntry = null; for (const entry of entries) { if (entry.isIntersecting) { bestEntry = entry; - break; // first intersecting is enough + break; } } if (!bestEntry) return; @@ -466,17 +584,17 @@ const DiffViewer = { if (!filename || filename === this._lastActiveFile) return; this._lastActiveFile = filename; - // Only toggle the changed items, not full scan + const prev = this.container.querySelector('.file-tree-file.active'); + if (prev) prev.classList.remove('active'); + const treeFiles = this.container.querySelectorAll('.file-tree-file'); for (const item of treeFiles) { const treeName = item.getAttribute('data-filename'); if (!treeName) continue; - const isActive = treeName === filename || treeName.endsWith('/' + filename) || filename.endsWith('/' + treeName); - if (isActive && !item.classList.contains('active')) { + if (treeName === filename || treeName.endsWith('/' + filename) || filename.endsWith('/' + treeName)) { item.classList.add('active'); item.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); - } else if (!isActive && item.classList.contains('active')) { - item.classList.remove('active'); + break; } } }); @@ -491,7 +609,8 @@ const DiffViewer = { }); }, - // Load and render diff from API + // ── Load from API ─────────────────────────────────────────────── + async loadDiff(repoId, base, head, options = {}) { if (!this.container) return; @@ -514,7 +633,6 @@ const DiffViewer = { } }, - // Load per-file diffs async loadFiles(repoId, base, head) { if (!this.container) return; @@ -532,39 +650,19 @@ const DiffViewer = { } }, - // Switch view mode (side-by-side or line-by-line) — only rebuild diff content, not the whole layout + // ── View mode switch ──────────────────────────────────────────── + setViewMode(mode) { if (!this.container || !this.rawDiff) return; this.currentMode = mode; - - const outputFormat = mode === 'unified' ? 'line-by-line' : 'side-by-side'; - const config = { - drawFileList: false, - fileListToggle: false, - fileContentToggle: true, - matching: 'lines', - outputFormat: outputFormat, - synchronisedScroll: true, - highlight: true, - renderNothingWhenEmpty: false, - }; - - const diffContent = this.container.querySelector('#diff-content'); - if (!diffContent) { - // Fallback: full re-render - this.renderDiff(this.rawDiff, { outputFormat }); - return; - } - - // Only rebuild the diff content area - diffContent.innerHTML = Diff2Html.html(this.rawDiff, config); - this._annotateFileSections(); - this._buildFileIndex(); - this._setupScrollSpy(); - this._setupLazyHighlight(); + // Full incremental re-render with new format + this.renderDiff(this.rawDiff, { + outputFormat: mode === 'unified' ? 'line-by-line' : 'side-by-side', + }); }, - // Toggle file list visibility + // ── Toggle helpers ────────────────────────────────────────────── + toggleFileList() { const sidebar = this.container.querySelector('.diff-sidebar'); if (sidebar) { @@ -572,36 +670,35 @@ const DiffViewer = { } }, - // Expand/collapse all files toggleAllFiles(expand) { const fileContents = this.container.querySelectorAll('.d2h-file-wrapper'); fileContents.forEach(file => { const content = file.querySelector('.d2h-file-diff'); if (content) { content.style.display = expand ? 'block' : 'none'; + file.classList.toggle('d2h-collapsed', !expand); } }); }, - // Insert a review suggestion inline at a specific line + // ── Insert suggestion (uses lazy index) ───────────────────────── + insertSuggestion(filename, line, side, severity, content, suggestionId) { const diffContent = this.container.querySelector('#diff-content'); if (!diffContent) return; - // Use pre-built index for O(1) file lookup - if (!this._fileIndex) return; - - let fileEntry = null; - // Try exact match first, then suffix match - for (const [key, val] of this._fileIndex) { - if (key === filename || key.endsWith('/' + filename) || filename.endsWith('/' + key)) { - fileEntry = val; - break; - } - } + // Ensure this file's index is built + const fileEntry = this._ensureFileIndexed(filename); if (!fileEntry) return; - // O(1) row lookup via index + // Expand the file if collapsed (so the row is visible) + const fileContent = fileEntry.wrapper.querySelector('.d2h-file-diff'); + if (fileContent && fileContent.style.display === 'none') { + fileContent.style.display = 'block'; + fileEntry.wrapper.classList.remove('d2h-collapsed'); + } + + // O(1) row lookup const rowKey = side + '-' + line; const row = fileEntry.rows.get(rowKey); if (!row) return; @@ -632,7 +729,6 @@ const DiffViewer = {
`; - // Insert after the current row row.parentNode.insertBefore(suggestionRow, row.nextSibling); } }; diff --git a/templates/pages/review.html b/templates/pages/review.html index c5c0fee..5975a5d 100644 --- a/templates/pages/review.html +++ b/templates/pages/review.html @@ -280,12 +280,14 @@ if (!container) return; DiffViewer.init('review-diff-container'); - DiffViewer.renderDiff(diffString); - - // Insert suggestions inline - allSuggestions.forEach((s, i) => { - if (s.file && s.line) { - DiffViewer.insertSuggestion(s.file, s.line, s.side || 'right', s.severity, s.content, 'sug-' + i); + DiffViewer.renderDiff(diffString, { + onComplete() { + // Insert suggestions after all files are rendered + allSuggestions.forEach((s, i) => { + if (s.file && s.line) { + DiffViewer.insertSuggestion(s.file, s.line, s.side || 'right', s.severity, s.content, 'sug-' + i); + } + }); } }); }