From 1ec71e877cde1a2fdea23d60f2a7ef5a995d9b09 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 20 Nov 2025 00:08:38 -0800 Subject: [PATCH 01/17] Add pull request files line selections --- templates/repo/diff/blob_excerpt.tmpl | 8 +- templates/repo/diff/section_split.tmpl | 8 +- templates/repo/diff/section_unified.tmpl | 4 +- web_src/css/repo.css | 28 +++++ web_src/js/features/repo-diff.ts | 125 +++++++++++++++++++++++ 5 files changed, 163 insertions(+), 10 deletions(-) diff --git a/templates/repo/diff/blob_excerpt.tmpl b/templates/repo/diff/blob_excerpt.tmpl index c9aac6d61d84c..a9323d4778193 100644 --- a/templates/repo/diff/blob_excerpt.tmpl +++ b/templates/repo/diff/blob_excerpt.tmpl @@ -11,7 +11,7 @@ {{else}} {{$inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale}} - + {{if and $line.LeftIdx $inlineDiff.EscapeStatus.Escaped}}{{end}} {{if $line.LeftIdx}}{{end}} @@ -27,7 +27,7 @@ {{- end -}} - + {{if and $line.RightIdx $inlineDiff.EscapeStatus.Escaped}}{{end}} {{if $line.RightIdx}}{{end}} @@ -65,8 +65,8 @@ {{if eq .GetType 4}} {{$line.RenderBlobExcerptButtons $.FileNameHash $diffBlobExcerptData}} {{else}} - - + + {{end}} {{$inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale}} {{if $inlineDiff.EscapeStatus.Escaped}}{{end}} diff --git a/templates/repo/diff/section_split.tmpl b/templates/repo/diff/section_split.tmpl index ab23b1b934b7b..da5390978aa26 100644 --- a/templates/repo/diff/section_split.tmpl +++ b/templates/repo/diff/section_split.tmpl @@ -24,7 +24,7 @@ {{$match := index $section.Lines $line.Match}} {{- $leftDiff := ""}}{{if $line.LeftIdx}}{{$leftDiff = $section.GetComputedInlineDiffFor $line ctx.Locale}}{{end}} {{- $rightDiff := ""}}{{if $match.RightIdx}}{{$rightDiff = $section.GetComputedInlineDiffFor $match ctx.Locale}}{{end}} - + {{if $line.LeftIdx}}{{if $leftDiff.EscapeStatus.Escaped}}{{end}}{{end}} @@ -39,7 +39,7 @@ {{- end -}} - + {{if $match.RightIdx}}{{if $rightDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $match.RightIdx}}{{end}} @@ -56,7 +56,7 @@ {{else}} {{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale}} - + {{if $line.LeftIdx}}{{if $inlineDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $line.LeftIdx}}{{end}} @@ -71,7 +71,7 @@ {{- end -}} - + {{if $line.RightIdx}}{{if $inlineDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $line.RightIdx}}{{end}} diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index 908b14656e36e..ddd48b1d84e65 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -19,8 +19,8 @@ {{end}} {{else}} - - + + {{end}} {{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale -}} diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 9b70e0e6dbaa1..1d7fcdbe137e0 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -985,6 +985,14 @@ td .commit-summary { text-align: right; } +.repository .diff-file-box .code-diff .lines-num[data-line-num] { + cursor: pointer; +} + +.repository .diff-file-box .code-diff .lines-num[data-line-num]:hover { + color: var(--color-text-dark); +} + .repository .diff-file-box .code-diff tbody tr .lines-type-marker { width: 10px; min-width: 10px; @@ -996,6 +1004,26 @@ td .commit-summary { display: inline-block; } +.repository .diff-file-box .code-diff tr.active .lines-num, +.repository .diff-file-box .code-diff tr.active .lines-escape, +.repository .diff-file-box .code-diff tr.active .lines-type-marker, +.repository .diff-file-box .code-diff tr.active .lines-code { + background: var(--color-highlight-bg); +} + +.repository .diff-file-box .code-diff tr.active .lines-num { + position: relative; +} + +.repository .diff-file-box .code-diff tr.active .lines-num::after { + content: ""; + position: absolute; + left: 0; + width: 2px; + height: 100%; + background: var(--color-highlight-fg); +} + .repository .diff-file-box .code-diff-split .tag-code .lines-code code.code-inner { padding-left: 10px !important; } diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 6f5cb2f63ba1a..c688c1826ea55 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -12,6 +12,130 @@ import {invertFileFolding} from './file-fold.ts'; import {parseDom, sleep} from '../utils.ts'; import {registerGlobalSelectorFunc} from '../modules/observer.ts'; +const diffLineNumberCellSelector = '#diff-file-boxes .code-diff td.lines-num[data-line-num]'; +const diffAnchorSuffixRegex = /([LR])(\d+)$/; +const diffHashRangeRegex = /^(diff-[0-9a-f]+)([LR]\d+)(?:-([LR]\d+))?$/i; + +type DiffAnchorSide = 'L' | 'R'; +type DiffAnchorInfo = {anchor: string, fragment: string, side: DiffAnchorSide, line: number}; +type DiffSelectionState = DiffAnchorInfo & {container: HTMLElement}; + +let diffSelectionStart: DiffSelectionState | null = null; + +function changeHash(hash: string) { + if (window.history.pushState) { + window.history.pushState(null, null, hash); + } else { + window.location.hash = hash; + } +} + +function parseDiffAnchor(anchor: string | null): DiffAnchorInfo | null { + if (!anchor || !anchor.startsWith('diff-')) return null; + const suffixMatch = diffAnchorSuffixRegex.exec(anchor); + if (!suffixMatch) return null; + const line = Number.parseInt(suffixMatch[2]); + if (Number.isNaN(line)) return null; + const fragment = anchor.slice(0, -suffixMatch[0].length); + const side = suffixMatch[1] as DiffAnchorSide; + return {anchor, fragment, side, line}; +} + +function applyDiffLineSelection(container: HTMLElement, fragment: string, side: DiffAnchorSide, startLine: number, endLine: number, options?: {updateHash?: boolean}): boolean { + const minLine = Math.min(startLine, endLine); + const maxLine = Math.max(startLine, endLine); + const selector = `.code-diff td.lines-num span[id^="${CSS.escape(fragment)}"]`; + const spans = Array.from(container.querySelectorAll(selector)); + const matches = spans.filter((span) => { + const info = parseDiffAnchor(span.id); + if (!info || info.side !== side) return false; + return info.line >= minLine && info.line <= maxLine; + }); + if (!matches.length) return false; + + for (const tr of document.querySelectorAll('.code-diff tr.active')) { + tr.classList.remove('active'); + } + for (const span of matches) { + span.closest('tr')?.classList.add('active'); + } + + if (options?.updateHash !== false) { + const startAnchor = `${fragment}${side}${minLine}`; + const endAnchor = `${fragment}${side}${maxLine}`; + const hashValue = minLine === maxLine ? startAnchor : `${startAnchor}-${endAnchor}`; + changeHash(`#${hashValue}`); + } + return true; +} + +type DiffHashRange = {fragment: string, side: DiffAnchorSide, startLine: number, endLine: number}; + +function parseDiffHashRange(hashValue: string): DiffHashRange | null { + if (!hashValue.startsWith('diff-')) return null; + const match = diffHashRangeRegex.exec(hashValue); + if (!match) return null; + const startInfo = parseDiffAnchor(`${match[1]}${match[2]}`); + if (!startInfo) return null; + let endLine = startInfo.line; + if (match[3]) { + const endInfo = parseDiffAnchor(`${match[1]}${match[3]}`); + if (!endInfo || endInfo.side !== startInfo.side) { + return {fragment: startInfo.fragment, side: startInfo.side, startLine: startInfo.line, endLine: startInfo.line}; + } + endLine = endInfo.line; + } + return { + fragment: startInfo.fragment, + side: startInfo.side, + startLine: startInfo.line, + endLine, + }; +} + +function highlightDiffSelectionFromHash() { + const {hash} = window.location; + if (!hash || !hash.startsWith('#diff-')) return; + const range = parseDiffHashRange(hash.substring(1)); + if (!range) return; + const targetId = `${range.fragment}${range.side}${range.startLine}`; + const target = document.querySelector(`#${CSS.escape(targetId)}`); + if (!target) return; + const container = target.closest('.diff-file-box'); + if (!container) return; + applyDiffLineSelection(container, range.fragment, range.side, range.startLine, range.endLine, {updateHash: false}); + diffSelectionStart = null; +} + +function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { + const span = cell.querySelector('span[id^="diff-"]'); + const info = parseDiffAnchor(span?.id ?? null); + if (!info) return; + const container = cell.closest('.diff-file-box'); + if (!container) return; + + let rangeStart: DiffAnchorInfo = info; + if (e.shiftKey && diffSelectionStart && + diffSelectionStart.container === container && + diffSelectionStart.fragment === info.fragment && + diffSelectionStart.side === info.side) { + rangeStart = diffSelectionStart; + } + + if (applyDiffLineSelection(container, rangeStart.fragment, rangeStart.side, rangeStart.line, info.line)) { + diffSelectionStart = {...info, container}; + window.getSelection().removeAllRanges(); + } +} + +function initDiffLineSelection() { + addDelegatedEventListener(document, 'click', diffLineNumberCellSelector, (cell, e) => { + handleDiffLineNumberClick(cell, e); + }); + window.addEventListener('hashchange', highlightDiffSelectionFromHash); + highlightDiffSelectionFromHash(); +} + function initRepoDiffFileBox(el: HTMLElement) { // switch between "rendered" and "source", for image and CSV files queryElems(el, '.file-view-toggle', (btn) => btn.addEventListener('click', () => { @@ -283,6 +407,7 @@ export function initRepoDiffView() { initDiffHeaderPopup(); initViewedCheckboxListenerFor(); initExpandAndCollapseFilesButton(); + initDiffLineSelection(); initRepoDiffHashChangeListener(); registerGlobalSelectorFunc('#diff-file-boxes .diff-file-box', initRepoDiffFileBox); From c144b64e8231688a33c46e0d2f7288407837dbfc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 23 Nov 2025 16:44:52 -0800 Subject: [PATCH 02/17] Fix bug --- web_src/js/features/repo-diff.ts | 170 +++++++++++++++++++++++-------- 1 file changed, 128 insertions(+), 42 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index c688c1826ea55..9d04a3ac5664c 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -19,6 +19,7 @@ const diffHashRangeRegex = /^(diff-[0-9a-f]+)([LR]\d+)(?:-([LR]\d+))?$/i; type DiffAnchorSide = 'L' | 'R'; type DiffAnchorInfo = {anchor: string, fragment: string, side: DiffAnchorSide, line: number}; type DiffSelectionState = DiffAnchorInfo & {container: HTMLElement}; +type DiffSelectionRange = {fragment: string, startSide: DiffAnchorSide, startLine: number, endSide: DiffAnchorSide, endLine: number}; let diffSelectionStart: DiffSelectionState | null = null; @@ -41,15 +42,27 @@ function parseDiffAnchor(anchor: string | null): DiffAnchorInfo | null { return {anchor, fragment, side, line}; } -function applyDiffLineSelection(container: HTMLElement, fragment: string, side: DiffAnchorSide, startLine: number, endLine: number, options?: {updateHash?: boolean}): boolean { - const minLine = Math.min(startLine, endLine); - const maxLine = Math.max(startLine, endLine); - const selector = `.code-diff td.lines-num span[id^="${CSS.escape(fragment)}"]`; +function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRange, options?: {updateHash?: boolean}): boolean { + const selector = `.code-diff td.lines-num span[id^="${CSS.escape(range.fragment)}"]`; const spans = Array.from(container.querySelectorAll(selector)); const matches = spans.filter((span) => { const info = parseDiffAnchor(span.id); - if (!info || info.side !== side) return false; - return info.line >= minLine && info.line <= maxLine; + if (!info) return false; + // For same side selection + if (range.startSide === range.endSide) { + if (info.side !== range.startSide) return false; + const minLine = Math.min(range.startLine, range.endLine); + const maxLine = Math.max(range.startLine, range.endLine); + return info.line >= minLine && info.line <= maxLine; + } + // For cross-side selection (L to R or R to L) + if (info.side === range.startSide) { + return info.line === range.startLine; + } + if (info.side === range.endSide) { + return info.line === range.endLine; + } + return false; }); if (!matches.length) return false; @@ -61,50 +74,86 @@ function applyDiffLineSelection(container: HTMLElement, fragment: string, side: } if (options?.updateHash !== false) { - const startAnchor = `${fragment}${side}${minLine}`; - const endAnchor = `${fragment}${side}${maxLine}`; - const hashValue = minLine === maxLine ? startAnchor : `${startAnchor}-${endAnchor}`; + const startAnchor = `${range.fragment}${range.startSide}${range.startLine}`; + const hashValue = (range.startSide === range.endSide && range.startLine === range.endLine) ? + startAnchor : + `${startAnchor}-${range.endSide}${range.endLine}`; changeHash(`#${hashValue}`); } return true; } -type DiffHashRange = {fragment: string, side: DiffAnchorSide, startLine: number, endLine: number}; - -function parseDiffHashRange(hashValue: string): DiffHashRange | null { +function parseDiffHashRange(hashValue: string): DiffSelectionRange | null { if (!hashValue.startsWith('diff-')) return null; const match = diffHashRangeRegex.exec(hashValue); if (!match) return null; const startInfo = parseDiffAnchor(`${match[1]}${match[2]}`); if (!startInfo) return null; + let endSide = startInfo.side; let endLine = startInfo.line; if (match[3]) { const endInfo = parseDiffAnchor(`${match[1]}${match[3]}`); - if (!endInfo || endInfo.side !== startInfo.side) { - return {fragment: startInfo.fragment, side: startInfo.side, startLine: startInfo.line, endLine: startInfo.line}; + if (!endInfo) { + return {fragment: startInfo.fragment, startSide: startInfo.side, startLine: startInfo.line, endSide: startInfo.side, endLine: startInfo.line}; } + endSide = endInfo.side; endLine = endInfo.line; } return { fragment: startInfo.fragment, - side: startInfo.side, + startSide: startInfo.side, startLine: startInfo.line, + endSide, endLine, }; } -function highlightDiffSelectionFromHash() { +async function highlightDiffSelectionFromHash(): Promise { const {hash} = window.location; - if (!hash || !hash.startsWith('#diff-')) return; + if (!hash || !hash.startsWith('#diff-')) return false; const range = parseDiffHashRange(hash.substring(1)); - if (!range) return; - const targetId = `${range.fragment}${range.side}${range.startLine}`; - const target = document.querySelector(`#${CSS.escape(targetId)}`); - if (!target) return; - const container = target.closest('.diff-file-box'); - if (!container) return; - applyDiffLineSelection(container, range.fragment, range.side, range.startLine, range.endLine, {updateHash: false}); - diffSelectionStart = null; + if (!range) return false; + const targetId = `${range.fragment}${range.startSide}${range.startLine}`; + + // Wait for the target element to be available (in case it needs to be loaded) + const targetSpan = document.querySelector(`#${CSS.escape(targetId)}`); + if (!targetSpan) { + // Target not found - it might need to be loaded via "show more files" + // Return false to let onLocationHashChange handle the loading + return false; + } + + const container = targetSpan.closest('.diff-file-box'); + if (!container) return false; + + // Check if the file is collapsed and expand it if needed + const fileContent = container.querySelector('.file-content'); + if (fileContent?.classList.contains('folded')) { + const foldBtn = container.querySelector('.fold-file'); + if (foldBtn) { + invertFileFolding(fileContent, foldBtn); + // Wait a bit for the expansion animation + await sleep(100); + } + } + + if (!applyDiffLineSelection(container, range, {updateHash: false})) return false; + diffSelectionStart = { + anchor: targetId, + fragment: range.fragment, + side: range.startSide, + line: range.startLine, + container, + }; + + // Scroll to the first selected line (scroll to the tr element, not the span) + // The span is an inline element inside td, we need to scroll to the tr for better visibility + await sleep(10); + const targetTr = targetSpan.closest('tr'); + if (targetTr) { + targetTr.scrollIntoView({behavior: 'smooth', block: 'center'}); + } + return true; } function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { @@ -117,12 +166,19 @@ function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { let rangeStart: DiffAnchorInfo = info; if (e.shiftKey && diffSelectionStart && diffSelectionStart.container === container && - diffSelectionStart.fragment === info.fragment && - diffSelectionStart.side === info.side) { + diffSelectionStart.fragment === info.fragment) { rangeStart = diffSelectionStart; } - if (applyDiffLineSelection(container, rangeStart.fragment, rangeStart.side, rangeStart.line, info.line)) { + const range: DiffSelectionRange = { + fragment: info.fragment, + startSide: rangeStart.side, + startLine: rangeStart.line, + endSide: info.side, + endLine: info.line, + }; + + if (applyDiffLineSelection(container, range)) { diffSelectionStart = {...info, container}; window.getSelection().removeAllRanges(); } @@ -132,7 +188,9 @@ function initDiffLineSelection() { addDelegatedEventListener(document, 'click', diffLineNumberCellSelector, (cell, e) => { handleDiffLineNumberClick(cell, e); }); - window.addEventListener('hashchange', highlightDiffSelectionFromHash); + window.addEventListener('hashchange', () => { + highlightDiffSelectionFromHash(); + }); highlightDiffSelectionFromHash(); } @@ -272,12 +330,14 @@ function initDiffHeaderPopup() { } // Will be called when the show more (files) button has been pressed -function onShowMoreFiles() { +async function onShowMoreFiles() { // TODO: replace these calls with the "observer.ts" methods initRepoIssueContentHistory(); initViewedCheckboxListenerFor(); initImageDiff(); initDiffHeaderPopup(); + // Re-apply hash selection in case the target was just loaded + await highlightDiffSelectionFromHash(); } async function loadMoreFiles(btn: Element): Promise { @@ -348,20 +408,46 @@ async function onLocationHashChange() { const attrAutoScrollRunning = 'data-auto-scroll-running'; if (document.body.hasAttribute(attrAutoScrollRunning)) return; - const targetElementId = currentHash.substring(1); - while (currentHash === window.location.hash) { - // use getElementById to avoid querySelector throws an error when the hash is invalid - // eslint-disable-next-line unicorn/prefer-query-selector - const targetElement = document.getElementById(targetElementId); - if (targetElement) { - // need to change hash to re-trigger ":target" CSS selector, let's manually scroll to it - targetElement.scrollIntoView(); - document.body.setAttribute(attrAutoScrollRunning, 'true'); - window.location.hash = ''; - window.location.hash = currentHash; - setTimeout(() => document.body.removeAttribute(attrAutoScrollRunning), 0); + // Check if this is a diff line selection hash (e.g., #diff-xxxL10 or #diff-xxxL10-R20) + const hashValue = currentHash.substring(1); + const range = parseDiffHashRange(hashValue); + if (range) { + // This is a line selection hash, try to highlight it first + const success = await highlightDiffSelectionFromHash(); + if (success) { + // Successfully highlighted and scrolled, we're done return; } + // If not successful, fall through to load more files + } + + const targetElementId = hashValue; + while (currentHash === window.location.hash) { + // For line selections, check the range-based target + let targetElement; + if (range) { + const targetId = `${range.fragment}${range.startSide}${range.startLine}`; + // eslint-disable-next-line unicorn/prefer-query-selector + targetElement = document.getElementById(targetId); + if (targetElement) { + // Try again to highlight and scroll now that the element is loaded + await highlightDiffSelectionFromHash(); + return; + } + } else { + // use getElementById to avoid querySelector throws an error when the hash is invalid + // eslint-disable-next-line unicorn/prefer-query-selector + targetElement = document.getElementById(targetElementId); + if (targetElement) { + // need to change hash to re-trigger ":target" CSS selector, let's manually scroll to it + targetElement.scrollIntoView(); + document.body.setAttribute(attrAutoScrollRunning, 'true'); + window.location.hash = ''; + window.location.hash = currentHash; + setTimeout(() => document.body.removeAttribute(attrAutoScrollRunning), 0); + return; + } + } // If looking for a hidden comment, try to expand the section that contains it const issueCommentPrefix = '#issuecomment-'; From 373b530a8c934e204047266873f9a7bdb706d9bc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 23 Nov 2025 17:12:06 -0800 Subject: [PATCH 03/17] Fix unfold when loading with line selections --- web_src/js/features/repo-diff.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 9d04a3ac5664c..23c271ddfd3a3 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -8,7 +8,7 @@ import {showErrorToast} from '../modules/toast.ts'; import {submitEventSubmitter, queryElemSiblings, hideElem, showElem, animateOnce, addDelegatedEventListener, createElementFromHTML, queryElems} from '../utils/dom.ts'; import {POST, GET} from '../modules/fetch.ts'; import {createTippy} from '../modules/tippy.ts'; -import {invertFileFolding} from './file-fold.ts'; +import {invertFileFolding, setFileFolding} from './file-fold.ts'; import {parseDom, sleep} from '../utils.ts'; import {registerGlobalSelectorFunc} from '../modules/observer.ts'; @@ -127,11 +127,11 @@ async function highlightDiffSelectionFromHash(): Promise { if (!container) return false; // Check if the file is collapsed and expand it if needed - const fileContent = container.querySelector('.file-content'); - if (fileContent?.classList.contains('folded')) { + if (container.getAttribute('data-folded') === 'true') { const foldBtn = container.querySelector('.fold-file'); if (foldBtn) { - invertFileFolding(fileContent, foldBtn); + // Expand the file using the setFileFolding utility + setFileFolding(container, foldBtn, false); // Wait a bit for the expansion animation await sleep(100); } From 89989c5bde88d678e9ee8edf2877eb7007ecd7ae Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 23 Nov 2025 17:31:33 -0800 Subject: [PATCH 04/17] Support select the blank position of the line numbers --- web_src/js/features/repo-diff.ts | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 23c271ddfd3a3..3f689cf148960 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -57,10 +57,10 @@ function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRang } // For cross-side selection (L to R or R to L) if (info.side === range.startSide) { - return info.line === range.startLine; + return info.line >= range.startLine; } if (info.side === range.endSide) { - return info.line === range.endLine; + return info.line <= range.endLine; } return false; }); @@ -157,9 +157,25 @@ async function highlightDiffSelectionFromHash(): Promise { } function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { - const span = cell.querySelector('span[id^="diff-"]'); - const info = parseDiffAnchor(span?.id ?? null); - if (!info) return; + let span = cell.querySelector('span[id^="diff-"]'); + let info = parseDiffAnchor(span?.id ?? null); + + // If clicked cell has no line number (e.g., clicking on the empty side of a deletion/addition), + // try to find the line number from the sibling cell on the same row + if (!info) { + const row = cell.closest('tr'); + if (!row) return; + // Find the other line number cell in the same row + const siblingCell = cell.classList.contains('lines-num-old') ? + row.querySelector('td.lines-num-new') : + row.querySelector('td.lines-num-old'); + if (siblingCell) { + span = siblingCell.querySelector('span[id^="diff-"]'); + info = parseDiffAnchor(span?.id ?? null); + } + if (!info) return; + } + const container = cell.closest('.diff-file-box'); if (!container) return; From 35b8f11c345ffab416739017647244abe6905238 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 23 Nov 2025 17:42:12 -0800 Subject: [PATCH 05/17] Fix bug when selecting plain lines --- web_src/js/features/repo-diff.ts | 54 ++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 3f689cf148960..25f50cbd94f2d 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -43,34 +43,40 @@ function parseDiffAnchor(anchor: string | null): DiffAnchorInfo | null { } function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRange, options?: {updateHash?: boolean}): boolean { - const selector = `.code-diff td.lines-num span[id^="${CSS.escape(range.fragment)}"]`; - const spans = Array.from(container.querySelectorAll(selector)); - const matches = spans.filter((span) => { - const info = parseDiffAnchor(span.id); - if (!info) return false; - // For same side selection - if (range.startSide === range.endSide) { - if (info.side !== range.startSide) return false; - const minLine = Math.min(range.startLine, range.endLine); - const maxLine = Math.max(range.startLine, range.endLine); - return info.line >= minLine && info.line <= maxLine; - } - // For cross-side selection (L to R or R to L) - if (info.side === range.startSide) { - return info.line >= range.startLine; - } - if (info.side === range.endSide) { - return info.line <= range.endLine; - } - return false; - }); - if (!matches.length) return false; + // Find the start and end anchor elements + const startId = `${range.fragment}${range.startSide}${range.startLine}`; + const endId = `${range.fragment}${range.endSide}${range.endLine}`; + const startSpan = container.querySelector(`#${CSS.escape(startId)}`); + const endSpan = container.querySelector(`#${CSS.escape(endId)}`); + + if (!startSpan || !endSpan) return false; + const startTr = startSpan.closest('tr'); + const endTr = endSpan.closest('tr'); + if (!startTr || !endTr) return false; + + // Clear previous selection for (const tr of document.querySelectorAll('.code-diff tr.active')) { tr.classList.remove('active'); } - for (const span of matches) { - span.closest('tr')?.classList.add('active'); + + // Get all rows in the diff section + const allRows = Array.from(container.querySelectorAll('.code-diff tbody tr')); + const startIndex = allRows.indexOf(startTr); + const endIndex = allRows.indexOf(endTr); + + if (startIndex === -1 || endIndex === -1) return false; + + // Select all rows between start and end (inclusive) + const minIndex = Math.min(startIndex, endIndex); + const maxIndex = Math.max(startIndex, endIndex); + + for (let i = minIndex; i <= maxIndex; i++) { + const row = allRows[i]; + // Only select rows that are actual diff lines (not comment rows, etc.) + if (row.querySelector('td.lines-num')) { + row.classList.add('active'); + } } if (options?.updateHash !== false) { From 5580cc7bb5b091ff666ed3e3d8db7d34fbdb5646 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 24 Nov 2025 09:25:08 -0800 Subject: [PATCH 06/17] Fix bug --- web_src/js/features/repo-diff-selection.ts | 239 +++++++++++++++++++++ web_src/js/features/repo-diff.ts | 207 +----------------- 2 files changed, 241 insertions(+), 205 deletions(-) create mode 100644 web_src/js/features/repo-diff-selection.ts diff --git a/web_src/js/features/repo-diff-selection.ts b/web_src/js/features/repo-diff-selection.ts new file mode 100644 index 0000000000000..041a5d0645337 --- /dev/null +++ b/web_src/js/features/repo-diff-selection.ts @@ -0,0 +1,239 @@ +import {addDelegatedEventListener} from '../utils/dom.ts'; +import {sleep} from '../utils.ts'; +import {setFileFolding} from './file-fold.ts'; + +const diffLineNumberCellSelector = '#diff-file-boxes .code-diff td.lines-num[data-line-num]'; +const diffAnchorSuffixRegex = /([LR])(\d+)$/; +const diffHashRangeRegex = /^(diff-[0-9a-f]+)([LR]\d+)(?:-([LR]\d+))?$/i; + +type DiffAnchorSide = 'L' | 'R'; +type DiffAnchorInfo = {anchor: string, fragment: string, side: DiffAnchorSide, line: number}; +type DiffSelectionState = DiffAnchorInfo & {container: HTMLElement}; +type DiffSelectionRange = {fragment: string, startSide: DiffAnchorSide, startLine: number, endSide: DiffAnchorSide, endLine: number}; + +let diffSelectionStart: DiffSelectionState | null = null; + +function changeHash(hash: string) { + if (window.history.pushState) { + window.history.pushState(null, null, hash); + } else { + window.location.hash = hash; + } +} + +function parseDiffAnchor(anchor: string | null): DiffAnchorInfo | null { + if (!anchor || !anchor.startsWith('diff-')) return null; + const suffixMatch = diffAnchorSuffixRegex.exec(anchor); + if (!suffixMatch) return null; + const line = Number.parseInt(suffixMatch[2]); + if (Number.isNaN(line)) return null; + const fragment = anchor.slice(0, -suffixMatch[0].length); + const side = suffixMatch[1] as DiffAnchorSide; + return {anchor, fragment, side, line}; +} + +function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRange, options?: {updateHash?: boolean}): boolean { + // Find the start and end anchor elements + const startId = `${range.fragment}${range.startSide}${range.startLine}`; + const endId = `${range.fragment}${range.endSide}${range.endLine}`; + const startSpan = container.querySelector(`#${CSS.escape(startId)}`); + const endSpan = container.querySelector(`#${CSS.escape(endId)}`); + + if (!startSpan || !endSpan) return false; + + const startTr = startSpan.closest('tr'); + const endTr = endSpan.closest('tr'); + if (!startTr || !endTr) return false; + + // Clear previous selection + for (const tr of document.querySelectorAll('.code-diff tr.active')) { + tr.classList.remove('active'); + } + + // Get all rows in the diff section + const allRows = Array.from(container.querySelectorAll('.code-diff tbody tr')); + const startIndex = allRows.indexOf(startTr); + const endIndex = allRows.indexOf(endTr); + + if (startIndex === -1 || endIndex === -1) return false; + + // Select all rows between start and end (inclusive) + const minIndex = Math.min(startIndex, endIndex); + const maxIndex = Math.max(startIndex, endIndex); + + for (let i = minIndex; i <= maxIndex; i++) { + const row = allRows[i]; + // Only select rows that are actual diff lines (not comment rows, expansion buttons, etc.) + // Skip rows with data-line-type="4" which are code expansion buttons + if (row.querySelector('td.lines-num') && row.getAttribute('data-line-type') !== '4') { + row.classList.add('active'); + } + } + + if (options?.updateHash !== false) { + const startAnchor = `${range.fragment}${range.startSide}${range.startLine}`; + const hashValue = (range.startSide === range.endSide && range.startLine === range.endLine) ? + startAnchor : + `${startAnchor}-${range.endSide}${range.endLine}`; + changeHash(`#${hashValue}`); + } + return true; +} + +export function parseDiffHashRange(hashValue: string): DiffSelectionRange | null { + if (!hashValue.startsWith('diff-')) return null; + const match = diffHashRangeRegex.exec(hashValue); + if (!match) return null; + const startInfo = parseDiffAnchor(`${match[1]}${match[2]}`); + if (!startInfo) return null; + let endSide = startInfo.side; + let endLine = startInfo.line; + if (match[3]) { + const endInfo = parseDiffAnchor(`${match[1]}${match[3]}`); + if (!endInfo) { + return {fragment: startInfo.fragment, startSide: startInfo.side, startLine: startInfo.line, endSide: startInfo.side, endLine: startInfo.line}; + } + endSide = endInfo.side; + endLine = endInfo.line; + } + return { + fragment: startInfo.fragment, + startSide: startInfo.side, + startLine: startInfo.line, + endSide, + endLine, + }; +} + +export async function highlightDiffSelectionFromHash(): Promise { + const {hash} = window.location; + if (!hash || !hash.startsWith('#diff-')) return false; + const range = parseDiffHashRange(hash.substring(1)); + if (!range) return false; + const targetId = `${range.fragment}${range.startSide}${range.startLine}`; + + // Wait for the target element to be available (in case it needs to be loaded) + const targetSpan = document.querySelector(`#${CSS.escape(targetId)}`); + if (!targetSpan) { + // Target not found - it might need to be loaded via "show more files" + // Return false to let onLocationHashChange handle the loading + return false; + } + + const container = targetSpan.closest('.diff-file-box'); + if (!container) return false; + + // Check if the file is collapsed and expand it if needed + if (container.getAttribute('data-folded') === 'true') { + const foldBtn = container.querySelector('.fold-file'); + if (foldBtn) { + // Expand the file using the setFileFolding utility + setFileFolding(container, foldBtn, false); + // Wait a bit for the expansion animation + await sleep(100); + } + } + + if (!applyDiffLineSelection(container, range, {updateHash: false})) return false; + diffSelectionStart = { + anchor: targetId, + fragment: range.fragment, + side: range.startSide, + line: range.startLine, + container, + }; + + // Scroll to the first selected line (scroll to the tr element, not the span) + // The span is an inline element inside td, we need to scroll to the tr for better visibility + await sleep(10); + const targetTr = targetSpan.closest('tr'); + if (targetTr) { + targetTr.scrollIntoView({behavior: 'smooth', block: 'center'}); + } + return true; +} + +function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { + let span = cell.querySelector('span[id^="diff-"]'); + let info = parseDiffAnchor(span?.id ?? null); + + // If clicked cell has no line number (e.g., clicking on the empty side of a deletion/addition), + // try to find the line number from the sibling cell on the same row + if (!info) { + const row = cell.closest('tr'); + if (!row) return; + // Find the other line number cell in the same row + const siblingCell = cell.classList.contains('lines-num-old') ? + row.querySelector('td.lines-num-new') : + row.querySelector('td.lines-num-old'); + if (siblingCell) { + span = siblingCell.querySelector('span[id^="diff-"]'); + info = parseDiffAnchor(span?.id ?? null); + } + if (!info) return; + } + + const container = cell.closest('.diff-file-box'); + if (!container) return; + + e.preventDefault(); + + // Check if clicking on a single already-selected line without shift key - deselect it + if (!e.shiftKey) { + const clickedRow = cell.closest('tr'); + if (clickedRow?.classList.contains('active')) { + // Check if this is a single-line selection by checking if it's the only selected line + const selectedRows = container.querySelectorAll('.code-diff tr.active'); + if (selectedRows.length === 1) { + // This is a single selected line, deselect it + clickedRow.classList.remove('active'); + diffSelectionStart = null; + // Remove hash from URL completely + if (window.history.pushState) { + window.history.pushState(null, null, window.location.pathname + window.location.search); + } else { + window.location.hash = ''; + } + window.getSelection().removeAllRanges(); + return; + } + } + } + + let rangeStart: DiffAnchorInfo = info; + if (e.shiftKey && diffSelectionStart && + diffSelectionStart.container === container && + diffSelectionStart.fragment === info.fragment) { + rangeStart = diffSelectionStart; + } + + const range: DiffSelectionRange = { + fragment: info.fragment, + startSide: rangeStart.side, + startLine: rangeStart.line, + endSide: info.side, + endLine: info.line, + }; + + if (applyDiffLineSelection(container, range)) { + diffSelectionStart = {...info, container}; + window.getSelection().removeAllRanges(); + } +} + +export function initDiffLineSelection() { + addDelegatedEventListener(document, 'click', diffLineNumberCellSelector, (cell, e) => { + if (e.defaultPrevented) return; + // Ignore clicks on or inside code-expander-buttons + const target = e.target as HTMLElement; + if (target.closest('.code-expander-button') || target.closest('.code-expander-buttons') || + target.closest('button, a, input, select, textarea, summary, [role="button"]')) { + return; + } + handleDiffLineNumberClick(cell, e); + }); + window.addEventListener('hashchange', () => { + highlightDiffSelectionFromHash(); + }); + highlightDiffSelectionFromHash(); +} diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 25f50cbd94f2d..73c77dc795432 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -8,213 +8,10 @@ import {showErrorToast} from '../modules/toast.ts'; import {submitEventSubmitter, queryElemSiblings, hideElem, showElem, animateOnce, addDelegatedEventListener, createElementFromHTML, queryElems} from '../utils/dom.ts'; import {POST, GET} from '../modules/fetch.ts'; import {createTippy} from '../modules/tippy.ts'; -import {invertFileFolding, setFileFolding} from './file-fold.ts'; +import {invertFileFolding} from './file-fold.ts'; import {parseDom, sleep} from '../utils.ts'; import {registerGlobalSelectorFunc} from '../modules/observer.ts'; - -const diffLineNumberCellSelector = '#diff-file-boxes .code-diff td.lines-num[data-line-num]'; -const diffAnchorSuffixRegex = /([LR])(\d+)$/; -const diffHashRangeRegex = /^(diff-[0-9a-f]+)([LR]\d+)(?:-([LR]\d+))?$/i; - -type DiffAnchorSide = 'L' | 'R'; -type DiffAnchorInfo = {anchor: string, fragment: string, side: DiffAnchorSide, line: number}; -type DiffSelectionState = DiffAnchorInfo & {container: HTMLElement}; -type DiffSelectionRange = {fragment: string, startSide: DiffAnchorSide, startLine: number, endSide: DiffAnchorSide, endLine: number}; - -let diffSelectionStart: DiffSelectionState | null = null; - -function changeHash(hash: string) { - if (window.history.pushState) { - window.history.pushState(null, null, hash); - } else { - window.location.hash = hash; - } -} - -function parseDiffAnchor(anchor: string | null): DiffAnchorInfo | null { - if (!anchor || !anchor.startsWith('diff-')) return null; - const suffixMatch = diffAnchorSuffixRegex.exec(anchor); - if (!suffixMatch) return null; - const line = Number.parseInt(suffixMatch[2]); - if (Number.isNaN(line)) return null; - const fragment = anchor.slice(0, -suffixMatch[0].length); - const side = suffixMatch[1] as DiffAnchorSide; - return {anchor, fragment, side, line}; -} - -function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRange, options?: {updateHash?: boolean}): boolean { - // Find the start and end anchor elements - const startId = `${range.fragment}${range.startSide}${range.startLine}`; - const endId = `${range.fragment}${range.endSide}${range.endLine}`; - const startSpan = container.querySelector(`#${CSS.escape(startId)}`); - const endSpan = container.querySelector(`#${CSS.escape(endId)}`); - - if (!startSpan || !endSpan) return false; - - const startTr = startSpan.closest('tr'); - const endTr = endSpan.closest('tr'); - if (!startTr || !endTr) return false; - - // Clear previous selection - for (const tr of document.querySelectorAll('.code-diff tr.active')) { - tr.classList.remove('active'); - } - - // Get all rows in the diff section - const allRows = Array.from(container.querySelectorAll('.code-diff tbody tr')); - const startIndex = allRows.indexOf(startTr); - const endIndex = allRows.indexOf(endTr); - - if (startIndex === -1 || endIndex === -1) return false; - - // Select all rows between start and end (inclusive) - const minIndex = Math.min(startIndex, endIndex); - const maxIndex = Math.max(startIndex, endIndex); - - for (let i = minIndex; i <= maxIndex; i++) { - const row = allRows[i]; - // Only select rows that are actual diff lines (not comment rows, etc.) - if (row.querySelector('td.lines-num')) { - row.classList.add('active'); - } - } - - if (options?.updateHash !== false) { - const startAnchor = `${range.fragment}${range.startSide}${range.startLine}`; - const hashValue = (range.startSide === range.endSide && range.startLine === range.endLine) ? - startAnchor : - `${startAnchor}-${range.endSide}${range.endLine}`; - changeHash(`#${hashValue}`); - } - return true; -} - -function parseDiffHashRange(hashValue: string): DiffSelectionRange | null { - if (!hashValue.startsWith('diff-')) return null; - const match = diffHashRangeRegex.exec(hashValue); - if (!match) return null; - const startInfo = parseDiffAnchor(`${match[1]}${match[2]}`); - if (!startInfo) return null; - let endSide = startInfo.side; - let endLine = startInfo.line; - if (match[3]) { - const endInfo = parseDiffAnchor(`${match[1]}${match[3]}`); - if (!endInfo) { - return {fragment: startInfo.fragment, startSide: startInfo.side, startLine: startInfo.line, endSide: startInfo.side, endLine: startInfo.line}; - } - endSide = endInfo.side; - endLine = endInfo.line; - } - return { - fragment: startInfo.fragment, - startSide: startInfo.side, - startLine: startInfo.line, - endSide, - endLine, - }; -} - -async function highlightDiffSelectionFromHash(): Promise { - const {hash} = window.location; - if (!hash || !hash.startsWith('#diff-')) return false; - const range = parseDiffHashRange(hash.substring(1)); - if (!range) return false; - const targetId = `${range.fragment}${range.startSide}${range.startLine}`; - - // Wait for the target element to be available (in case it needs to be loaded) - const targetSpan = document.querySelector(`#${CSS.escape(targetId)}`); - if (!targetSpan) { - // Target not found - it might need to be loaded via "show more files" - // Return false to let onLocationHashChange handle the loading - return false; - } - - const container = targetSpan.closest('.diff-file-box'); - if (!container) return false; - - // Check if the file is collapsed and expand it if needed - if (container.getAttribute('data-folded') === 'true') { - const foldBtn = container.querySelector('.fold-file'); - if (foldBtn) { - // Expand the file using the setFileFolding utility - setFileFolding(container, foldBtn, false); - // Wait a bit for the expansion animation - await sleep(100); - } - } - - if (!applyDiffLineSelection(container, range, {updateHash: false})) return false; - diffSelectionStart = { - anchor: targetId, - fragment: range.fragment, - side: range.startSide, - line: range.startLine, - container, - }; - - // Scroll to the first selected line (scroll to the tr element, not the span) - // The span is an inline element inside td, we need to scroll to the tr for better visibility - await sleep(10); - const targetTr = targetSpan.closest('tr'); - if (targetTr) { - targetTr.scrollIntoView({behavior: 'smooth', block: 'center'}); - } - return true; -} - -function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { - let span = cell.querySelector('span[id^="diff-"]'); - let info = parseDiffAnchor(span?.id ?? null); - - // If clicked cell has no line number (e.g., clicking on the empty side of a deletion/addition), - // try to find the line number from the sibling cell on the same row - if (!info) { - const row = cell.closest('tr'); - if (!row) return; - // Find the other line number cell in the same row - const siblingCell = cell.classList.contains('lines-num-old') ? - row.querySelector('td.lines-num-new') : - row.querySelector('td.lines-num-old'); - if (siblingCell) { - span = siblingCell.querySelector('span[id^="diff-"]'); - info = parseDiffAnchor(span?.id ?? null); - } - if (!info) return; - } - - const container = cell.closest('.diff-file-box'); - if (!container) return; - - let rangeStart: DiffAnchorInfo = info; - if (e.shiftKey && diffSelectionStart && - diffSelectionStart.container === container && - diffSelectionStart.fragment === info.fragment) { - rangeStart = diffSelectionStart; - } - - const range: DiffSelectionRange = { - fragment: info.fragment, - startSide: rangeStart.side, - startLine: rangeStart.line, - endSide: info.side, - endLine: info.line, - }; - - if (applyDiffLineSelection(container, range)) { - diffSelectionStart = {...info, container}; - window.getSelection().removeAllRanges(); - } -} - -function initDiffLineSelection() { - addDelegatedEventListener(document, 'click', diffLineNumberCellSelector, (cell, e) => { - handleDiffLineNumberClick(cell, e); - }); - window.addEventListener('hashchange', () => { - highlightDiffSelectionFromHash(); - }); - highlightDiffSelectionFromHash(); -} +import {parseDiffHashRange, highlightDiffSelectionFromHash, initDiffLineSelection} from './repo-diff-selection.ts'; function initRepoDiffFileBox(el: HTMLElement) { // switch between "rendered" and "source", for image and CSV files From 83d18d622552ece8af1927853064632e299115bb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 24 Nov 2025 10:03:00 -0800 Subject: [PATCH 07/17] Fix bug --- web_src/js/features/repo-diff-selection.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web_src/js/features/repo-diff-selection.ts b/web_src/js/features/repo-diff-selection.ts index 041a5d0645337..906bff3a02a9d 100644 --- a/web_src/js/features/repo-diff-selection.ts +++ b/web_src/js/features/repo-diff-selection.ts @@ -216,7 +216,9 @@ function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { }; if (applyDiffLineSelection(container, range)) { - diffSelectionStart = {...info, container}; + if (!e.shiftKey || !diffSelectionStart || diffSelectionStart.container !== container || diffSelectionStart.fragment !== info.fragment) { + diffSelectionStart = {...info, container}; + } window.getSelection().removeAllRanges(); } } From 45d2fbc0ebeae579fee858c494a42cbf79c75a25 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Nov 2025 09:29:05 -0800 Subject: [PATCH 08/17] fix selection css --- web_src/css/repo.css | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 1d7fcdbe137e0..2b2dd8460d08e 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -1016,12 +1016,18 @@ td .commit-summary { } .repository .diff-file-box .code-diff tr.active .lines-num::after { + display: none; +} + +.repository .diff-file-box .code-diff tr.active .lines-num:first-of-type::after { content: ""; position: absolute; left: 0; + top: 0; + bottom: 0; width: 2px; - height: 100%; background: var(--color-highlight-fg); + display: block; } .repository .diff-file-box .code-diff-split .tag-code .lines-code code.code-inner { From 41857b4fcd3c9419d6e734137acca128c6406fa8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 11 Dec 2025 14:12:33 -0800 Subject: [PATCH 09/17] some improvements --- templates/repo/diff/blob_excerpt.tmpl | 28 +++--- templates/repo/diff/section_split.tmpl | 12 ++- templates/repo/diff/section_unified.tmpl | 8 +- web_src/js/features/file-fold.ts | 47 +++++++++- web_src/js/features/repo-code.ts | 10 +-- web_src/js/features/repo-diff-selection.ts | 100 +++++++++++++-------- web_src/js/features/repo-diff.ts | 51 +++++------ 7 files changed, 161 insertions(+), 95 deletions(-) diff --git a/templates/repo/diff/blob_excerpt.tmpl b/templates/repo/diff/blob_excerpt.tmpl index a9323d4778193..c08ee935af770 100644 --- a/templates/repo/diff/blob_excerpt.tmpl +++ b/templates/repo/diff/blob_excerpt.tmpl @@ -11,7 +11,8 @@ {{else}} {{$inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale}} - + {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $.FileNameHash $line.LeftIdx) "" -}} + {{if and $line.LeftIdx $inlineDiff.EscapeStatus.Escaped}}{{end}} {{if $line.LeftIdx}}{{end}} @@ -27,7 +28,8 @@ {{- end -}} - + {{- $rightAnchor := Iif $line.RightIdx (printf "diff-%sR%d" $.FileNameHash $line.RightIdx) "" -}} + {{if and $line.RightIdx $inlineDiff.EscapeStatus.Escaped}}{{end}} {{if $line.RightIdx}}{{end}} @@ -59,16 +61,18 @@ {{end}} {{end}} -{{else}} - {{range $k, $line := $.section.Lines}} - - {{if eq .GetType 4}} - {{$line.RenderBlobExcerptButtons $.FileNameHash $diffBlobExcerptData}} - {{else}} - - - {{end}} - {{$inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale}} + {{else}} + {{range $k, $line := $.section.Lines}} + + {{if eq .GetType 4}} + {{$line.RenderBlobExcerptButtons $.FileNameHash $diffBlobExcerptData}} + {{else}} + {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $.FileNameHash $line.LeftIdx) "" -}} + + {{- $rightAnchor := Iif $line.RightIdx (printf "diff-%sR%d" $.FileNameHash $line.RightIdx) "" -}} + + {{end}} + {{$inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale}} {{if $inlineDiff.EscapeStatus.Escaped}}{{end}} diff --git a/templates/repo/diff/section_split.tmpl b/templates/repo/diff/section_split.tmpl index da5390978aa26..0cc8303d5b8b6 100644 --- a/templates/repo/diff/section_split.tmpl +++ b/templates/repo/diff/section_split.tmpl @@ -24,7 +24,8 @@ {{$match := index $section.Lines $line.Match}} {{- $leftDiff := ""}}{{if $line.LeftIdx}}{{$leftDiff = $section.GetComputedInlineDiffFor $line ctx.Locale}}{{end}} {{- $rightDiff := ""}}{{if $match.RightIdx}}{{$rightDiff = $section.GetComputedInlineDiffFor $match ctx.Locale}}{{end}} - + {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $file.NameHash $line.LeftIdx) "" -}} + {{if $line.LeftIdx}}{{if $leftDiff.EscapeStatus.Escaped}}{{end}}{{end}} @@ -39,7 +40,8 @@ {{- end -}} - + {{- $matchRightAnchor := Iif $match.RightIdx (printf "diff-%sR%d" $file.NameHash $match.RightIdx) "" -}} + {{if $match.RightIdx}}{{if $rightDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $match.RightIdx}}{{end}} @@ -56,7 +58,8 @@ {{else}} {{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale}} - + {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $file.NameHash $line.LeftIdx) "" -}} + {{if $line.LeftIdx}}{{if $inlineDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $line.LeftIdx}}{{end}} @@ -71,7 +74,8 @@ {{- end -}} - + {{- $rightAnchor := Iif $line.RightIdx (printf "diff-%sR%d" $file.NameHash $line.RightIdx) "" -}} + {{if $line.RightIdx}}{{if $inlineDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $line.RightIdx}}{{end}} diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index ddd48b1d84e65..b66206446cc82 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -19,9 +19,11 @@ {{end}} {{else}} - - - {{end}} + {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $file.NameHash $line.LeftIdx) "" -}} + + {{- $rightAnchor := Iif $line.RightIdx (printf "diff-%sR%d" $file.NameHash $line.RightIdx) "" -}} + + {{end}} {{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale -}} {{- if $inlineDiff.EscapeStatus.Escaped -}} diff --git a/web_src/js/features/file-fold.ts b/web_src/js/features/file-fold.ts index 74b36c0096c8b..fc68f9031fe22 100644 --- a/web_src/js/features/file-fold.ts +++ b/web_src/js/features/file-fold.ts @@ -1,19 +1,60 @@ import {svg} from '../svg.ts'; +function parseTransitionValue(value: string): number { + let max = 0; + for (const current of value.split(',')) { + const trimmed = current.trim(); + if (!trimmed) continue; + const isMs = trimmed.endsWith('ms'); + const numericPortion = Number.parseFloat(trimmed.replace(/ms|s$/u, '')); + if (Number.isNaN(numericPortion)) continue; + const duration = numericPortion * (isMs ? 1 : 1000); + max = Math.max(max, duration); + } + return max; +} + +function waitForTransitionEnd(element: Element): Promise { + if (!(element instanceof HTMLElement)) return Promise.resolve(); + const transitionTarget = element.querySelector('.diff-file-body') ?? element; + const styles = window.getComputedStyle(transitionTarget); + const transitionDuration = parseTransitionValue(styles.transitionDuration); + const transitionDelay = parseTransitionValue(styles.transitionDelay); + const total = transitionDuration + transitionDelay; + if (total === 0) return Promise.resolve(); + + return new Promise((resolve) => { + let resolved = false; + function cleanup() { + if (resolved) return; + resolved = true; + transitionTarget.removeEventListener('transitionend', onTransitionEnd); + resolve(); + } + function onTransitionEnd(event: TransitionEvent) { + if (event.target !== transitionTarget) return; + cleanup(); + } + transitionTarget.addEventListener('transitionend', onTransitionEnd); + window.setTimeout(cleanup, total + 50); + }); +} + // Hides the file if newFold is true, and shows it otherwise. The actual hiding is performed using CSS. // // The fold arrow is the icon displayed on the upper left of the file box, especially intended for components having the 'fold-file' class. // The file content box is the box that should be hidden or shown, especially intended for components having the 'file-content' class. // -export function setFileFolding(fileContentBox: Element, foldArrow: HTMLElement, newFold: boolean) { +export function setFileFolding(fileContentBox: Element, foldArrow: HTMLElement, newFold: boolean): Promise { foldArrow.innerHTML = svg(`octicon-chevron-${newFold ? 'right' : 'down'}`, 18); fileContentBox.setAttribute('data-folded', String(newFold)); if (newFold && fileContentBox.getBoundingClientRect().top < 0) { fileContentBox.scrollIntoView(); } + return waitForTransitionEnd(fileContentBox); } // Like `setFileFolding`, except that it automatically inverts the current file folding state. -export function invertFileFolding(fileContentBox:HTMLElement, foldArrow: HTMLElement) { - setFileFolding(fileContentBox, foldArrow, fileContentBox.getAttribute('data-folded') !== 'true'); +export function invertFileFolding(fileContentBox:HTMLElement, foldArrow: HTMLElement): Promise { + return setFileFolding(fileContentBox, foldArrow, fileContentBox.getAttribute('data-folded') !== 'true'); } diff --git a/web_src/js/features/repo-code.ts b/web_src/js/features/repo-code.ts index bf7fd762b0b34..c7e34f2fd705f 100644 --- a/web_src/js/features/repo-code.ts +++ b/web_src/js/features/repo-code.ts @@ -3,14 +3,6 @@ import {createTippy} from '../modules/tippy.ts'; import {toAbsoluteUrl} from '../utils.ts'; import {addDelegatedEventListener} from '../utils/dom.ts'; -function changeHash(hash: string) { - if (window.history.pushState) { - window.history.pushState(null, null, hash); - } else { - window.location.hash = hash; - } -} - // it selects the code lines defined by range: `L1-L3` (3 lines) or `L2` (singe line) function selectRange(range: string): Element { for (const el of document.querySelectorAll('.code-view tr.active')) el.classList.remove('active'); @@ -65,7 +57,7 @@ function selectRange(range: string): Element { for (let i = startLineNum - 1; i <= stopLineNum - 1 && i < elLineNums.length; i++) { elLineNums[i].closest('tr').classList.add('active'); } - changeHash(`#${range}`); + window.history.replaceState(null, '', `#${range}`); updateIssueHref(range); updateViewGitBlameFragment(range); updateCopyPermalinkUrl(range); diff --git a/web_src/js/features/repo-diff-selection.ts b/web_src/js/features/repo-diff-selection.ts index 906bff3a02a9d..7d4f70efebdc0 100644 --- a/web_src/js/features/repo-diff-selection.ts +++ b/web_src/js/features/repo-diff-selection.ts @@ -1,10 +1,10 @@ import {addDelegatedEventListener} from '../utils/dom.ts'; -import {sleep} from '../utils.ts'; import {setFileFolding} from './file-fold.ts'; const diffLineNumberCellSelector = '#diff-file-boxes .code-diff td.lines-num[data-line-num]'; const diffAnchorSuffixRegex = /([LR])(\d+)$/; const diffHashRangeRegex = /^(diff-[0-9a-f]+)([LR]\d+)(?:-([LR]\d+))?$/i; +export const diffAutoScrollAttr = 'data-auto-scroll-running'; type DiffAnchorSide = 'L' | 'R'; type DiffAnchorInfo = {anchor: string, fragment: string, side: DiffAnchorSide, line: number}; @@ -13,16 +13,20 @@ type DiffSelectionRange = {fragment: string, startSide: DiffAnchorSide, startLin let diffSelectionStart: DiffSelectionState | null = null; -function changeHash(hash: string) { - if (window.history.pushState) { - window.history.pushState(null, null, hash); - } else { - window.location.hash = hash; - } +function scrollDiffAnchorIntoView(targetElement: HTMLElement, currentHash: string) { + targetElement.scrollIntoView(); + document.body.setAttribute(diffAutoScrollAttr, 'true'); + window.location.hash = ''; + window.location.hash = currentHash; + setTimeout(() => document.body.removeAttribute(diffAutoScrollAttr), 0); +} + +function isDiffAnchorId(id: string | null): boolean { + return id !== null && id.startsWith('diff-'); } function parseDiffAnchor(anchor: string | null): DiffAnchorInfo | null { - if (!anchor || !anchor.startsWith('diff-')) return null; + if (!isDiffAnchorId(anchor)) return null; const suffixMatch = diffAnchorSuffixRegex.exec(anchor); if (!suffixMatch) return null; const line = Number.parseInt(suffixMatch[2]); @@ -32,7 +36,7 @@ function parseDiffAnchor(anchor: string | null): DiffAnchorInfo | null { return {anchor, fragment, side, line}; } -function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRange, options?: {updateHash?: boolean}): boolean { +function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRange): boolean { // Find the start and end anchor elements const startId = `${range.fragment}${range.startSide}${range.startLine}`; const endId = `${range.fragment}${range.endSide}${range.endLine}`; @@ -50,8 +54,10 @@ function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRang tr.classList.remove('active'); } - // Get all rows in the diff section - const allRows = Array.from(container.querySelectorAll('.code-diff tbody tr')); + // gather rows from the actual table that contains the selection to avoid missing hunks + const codeDiffTable = startSpan.closest('.code-diff'); + if (!codeDiffTable || !codeDiffTable.contains(endSpan)) return false; + const allRows = Array.from(codeDiffTable.querySelectorAll('tbody tr')); const startIndex = allRows.indexOf(startTr); const endIndex = allRows.indexOf(endTr); @@ -70,18 +76,25 @@ function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRang } } - if (options?.updateHash !== false) { - const startAnchor = `${range.fragment}${range.startSide}${range.startLine}`; - const hashValue = (range.startSide === range.endSide && range.startLine === range.endLine) ? - startAnchor : - `${startAnchor}-${range.endSide}${range.endLine}`; - changeHash(`#${hashValue}`); - } return true; } +function buildDiffHash(range: DiffSelectionRange): string { + const startAnchor = `${range.fragment}${range.startSide}${range.startLine}`; + if (range.startSide === range.endSide && range.startLine === range.endLine) { + return startAnchor; + } + return `${startAnchor}-${range.endSide}${range.endLine}`; +} + +function updateDiffHash(range: DiffSelectionRange) { + const hashValue = `#${buildDiffHash(range)}`; + if (window.location.hash === hashValue) return; + window.history.replaceState(null, '', hashValue); +} + export function parseDiffHashRange(hashValue: string): DiffSelectionRange | null { - if (!hashValue.startsWith('diff-')) return null; + if (!isDiffAnchorId(hashValue)) return null; const match = diffHashRangeRegex.exec(hashValue); if (!match) return null; const startInfo = parseDiffAnchor(`${match[1]}${match[2]}`); @@ -105,19 +118,36 @@ export function parseDiffHashRange(hashValue: string): DiffSelectionRange | null }; } +async function waitNextAnimationFrame() { + await new Promise((resolve) => requestAnimationFrame(() => resolve(undefined))); +} + export async function highlightDiffSelectionFromHash(): Promise { const {hash} = window.location; if (!hash || !hash.startsWith('#diff-')) return false; - const range = parseDiffHashRange(hash.substring(1)); - if (!range) return false; + const hashValue = hash.substring(1); + const range = parseDiffHashRange(hashValue); + if (!range) { + if (document.body.hasAttribute(diffAutoScrollAttr)) return false; + // eslint-disable-next-line unicorn/prefer-query-selector + const targetElement = document.getElementById(hashValue); + if (!targetElement) return false; + scrollDiffAnchorIntoView(targetElement, hash); + return true; + } const targetId = `${range.fragment}${range.startSide}${range.startLine}`; // Wait for the target element to be available (in case it needs to be loaded) - const targetSpan = document.querySelector(`#${CSS.escape(targetId)}`); + let targetSpan = document.querySelector(`#${CSS.escape(targetId)}`); if (!targetSpan) { - // Target not found - it might need to be loaded via "show more files" - // Return false to let onLocationHashChange handle the loading - return false; + // Flush pending DOM mutations (htmx, folding animations, etc.) before giving up + await waitNextAnimationFrame(); + targetSpan = document.querySelector(`#${CSS.escape(targetId)}`); + if (!targetSpan) { + // Target not found - it might need to be loaded via "show more files" + // Return false to let onLocationHashChange handle the loading + return false; + } } const container = targetSpan.closest('.diff-file-box'); @@ -127,14 +157,13 @@ export async function highlightDiffSelectionFromHash(): Promise { if (container.getAttribute('data-folded') === 'true') { const foldBtn = container.querySelector('.fold-file'); if (foldBtn) { - // Expand the file using the setFileFolding utility - setFileFolding(container, foldBtn, false); - // Wait a bit for the expansion animation - await sleep(100); + // Expand the file and wait for any transition to finish before selecting lines + await setFileFolding(container, foldBtn, false); } } - if (!applyDiffLineSelection(container, range, {updateHash: false})) return false; + if (!applyDiffLineSelection(container, range)) return false; + updateDiffHash(range); diffSelectionStart = { anchor: targetId, fragment: range.fragment, @@ -145,10 +174,10 @@ export async function highlightDiffSelectionFromHash(): Promise { // Scroll to the first selected line (scroll to the tr element, not the span) // The span is an inline element inside td, we need to scroll to the tr for better visibility - await sleep(10); + await waitNextAnimationFrame(); const targetTr = targetSpan.closest('tr'); if (targetTr) { - targetTr.scrollIntoView({behavior: 'smooth', block: 'center'}); + targetTr.scrollIntoView({block: 'center'}); } return true; } @@ -189,11 +218,7 @@ function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { clickedRow.classList.remove('active'); diffSelectionStart = null; // Remove hash from URL completely - if (window.history.pushState) { - window.history.pushState(null, null, window.location.pathname + window.location.search); - } else { - window.location.hash = ''; - } + window.history.replaceState(null, '', window.location.pathname + window.location.search); window.getSelection().removeAllRanges(); return; } @@ -216,6 +241,7 @@ function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { }; if (applyDiffLineSelection(container, range)) { + updateDiffHash(range); if (!e.shiftKey || !diffSelectionStart || diffSelectionStart.container !== container || diffSelectionStart.fragment !== info.fragment) { diffSelectionStart = {...info, container}; } diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 73c77dc795432..2e414a4f75dbf 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -11,7 +11,7 @@ import {createTippy} from '../modules/tippy.ts'; import {invertFileFolding} from './file-fold.ts'; import {parseDom, sleep} from '../utils.ts'; import {registerGlobalSelectorFunc} from '../modules/observer.ts'; -import {parseDiffHashRange, highlightDiffSelectionFromHash, initDiffLineSelection} from './repo-diff-selection.ts'; +import {parseDiffHashRange, highlightDiffSelectionFromHash, initDiffLineSelection, diffAutoScrollAttr} from './repo-diff-selection.ts'; function initRepoDiffFileBox(el: HTMLElement) { // switch between "rendered" and "source", for image and CSV files @@ -221,56 +221,53 @@ function initRepoDiffShowMore() { async function onLocationHashChange() { // try to scroll to the target element by the current hash const currentHash = window.location.hash; - if (!currentHash.startsWith('#diff-') && !currentHash.startsWith('#issuecomment-')) return; + const issueCommentPrefix = '#issuecomment-'; + const isDiffHash = currentHash.startsWith('#diff-'); + const isIssueCommentHash = currentHash.startsWith(issueCommentPrefix); + if (!isDiffHash && !isIssueCommentHash) return; // avoid reentrance when we are changing the hash to scroll and trigger ":target" selection - const attrAutoScrollRunning = 'data-auto-scroll-running'; - if (document.body.hasAttribute(attrAutoScrollRunning)) return; + if (document.body.hasAttribute(diffAutoScrollAttr)) return; - // Check if this is a diff line selection hash (e.g., #diff-xxxL10 or #diff-xxxL10-R20) const hashValue = currentHash.substring(1); - const range = parseDiffHashRange(hashValue); - if (range) { - // This is a line selection hash, try to highlight it first + let targetElementId = hashValue; + + if (isDiffHash) { const success = await highlightDiffSelectionFromHash(); if (success) { // Successfully highlighted and scrolled, we're done return; } - // If not successful, fall through to load more files + const range = parseDiffHashRange(hashValue); + if (range) { + targetElementId = `${range.fragment}${range.startSide}${range.startLine}`; + } } - const targetElementId = hashValue; while (currentHash === window.location.hash) { - // For line selections, check the range-based target - let targetElement; - if (range) { - const targetId = `${range.fragment}${range.startSide}${range.startLine}`; + if (isDiffHash) { // eslint-disable-next-line unicorn/prefer-query-selector - targetElement = document.getElementById(targetId); + const targetElement = document.getElementById(targetElementId); if (targetElement) { // Try again to highlight and scroll now that the element is loaded - await highlightDiffSelectionFromHash(); - return; + const success = await highlightDiffSelectionFromHash(); + if (success) return; } - } else { - // use getElementById to avoid querySelector throws an error when the hash is invalid + } else if (isIssueCommentHash) { // eslint-disable-next-line unicorn/prefer-query-selector - targetElement = document.getElementById(targetElementId); - if (targetElement) { - // need to change hash to re-trigger ":target" CSS selector, let's manually scroll to it - targetElement.scrollIntoView(); - document.body.setAttribute(attrAutoScrollRunning, 'true'); + const commentElement = document.getElementById(hashValue); + if (commentElement) { + commentElement.scrollIntoView(); + document.body.setAttribute(diffAutoScrollAttr, 'true'); window.location.hash = ''; window.location.hash = currentHash; - setTimeout(() => document.body.removeAttribute(attrAutoScrollRunning), 0); + setTimeout(() => document.body.removeAttribute(diffAutoScrollAttr), 0); return; } } // If looking for a hidden comment, try to expand the section that contains it - const issueCommentPrefix = '#issuecomment-'; - if (currentHash.startsWith(issueCommentPrefix)) { + if (isIssueCommentHash) { const commentId = currentHash.substring(issueCommentPrefix.length); const expandButton = document.querySelector(`.code-expander-button[data-hidden-comment-ids*=",${commentId},"]`); if (expandButton) { From 91fc2209b93462f5f126131463d437a7d5e9ab86 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 11 Dec 2025 14:19:44 -0800 Subject: [PATCH 10/17] Fix conflicts --- web_src/js/features/repo-diff-selection.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/web_src/js/features/repo-diff-selection.ts b/web_src/js/features/repo-diff-selection.ts index 7d4f70efebdc0..04ea1f9b20fb9 100644 --- a/web_src/js/features/repo-diff-selection.ts +++ b/web_src/js/features/repo-diff-selection.ts @@ -25,7 +25,7 @@ function isDiffAnchorId(id: string | null): boolean { return id !== null && id.startsWith('diff-'); } -function parseDiffAnchor(anchor: string | null): DiffAnchorInfo | null { +function parseDiffAnchor(anchor: string): DiffAnchorInfo | null { if (!isDiffAnchorId(anchor)) return null; const suffixMatch = diffAnchorSuffixRegex.exec(anchor); if (!suffixMatch) return null; @@ -184,7 +184,7 @@ export async function highlightDiffSelectionFromHash(): Promise { function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { let span = cell.querySelector('span[id^="diff-"]'); - let info = parseDiffAnchor(span?.id ?? null); + let info = parseDiffAnchor(span?.id ?? ''); // If clicked cell has no line number (e.g., clicking on the empty side of a deletion/addition), // try to find the line number from the sibling cell on the same row @@ -197,7 +197,7 @@ function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { row.querySelector('td.lines-num-old'); if (siblingCell) { span = siblingCell.querySelector('span[id^="diff-"]'); - info = parseDiffAnchor(span?.id ?? null); + info = parseDiffAnchor(span?.id ?? ''); } if (!info) return; } @@ -219,7 +219,7 @@ function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { diffSelectionStart = null; // Remove hash from URL completely window.history.replaceState(null, '', window.location.pathname + window.location.search); - window.getSelection().removeAllRanges(); + window.getSelection()?.removeAllRanges(); return; } } @@ -245,7 +245,7 @@ function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { if (!e.shiftKey || !diffSelectionStart || diffSelectionStart.container !== container || diffSelectionStart.fragment !== info.fragment) { diffSelectionStart = {...info, container}; } - window.getSelection().removeAllRanges(); + window.getSelection()?.removeAllRanges(); } } From ac5f96bd5f5fa7a877bb43f8e2741bb645ebf497 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 11 Dec 2025 14:22:37 -0800 Subject: [PATCH 11/17] Fix format --- templates/repo/diff/blob_excerpt.tmpl | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/templates/repo/diff/blob_excerpt.tmpl b/templates/repo/diff/blob_excerpt.tmpl index c08ee935af770..424231a50539e 100644 --- a/templates/repo/diff/blob_excerpt.tmpl +++ b/templates/repo/diff/blob_excerpt.tmpl @@ -61,18 +61,18 @@ {{end}} {{end}} - {{else}} - {{range $k, $line := $.section.Lines}} - - {{if eq .GetType 4}} - {{$line.RenderBlobExcerptButtons $.FileNameHash $diffBlobExcerptData}} - {{else}} - {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $.FileNameHash $line.LeftIdx) "" -}} - - {{- $rightAnchor := Iif $line.RightIdx (printf "diff-%sR%d" $.FileNameHash $line.RightIdx) "" -}} - - {{end}} - {{$inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale}} +{{else}} + {{range $k, $line := $.section.Lines}} + + {{if eq .GetType 4}} + {{$line.RenderBlobExcerptButtons $.FileNameHash $diffBlobExcerptData}} + {{else}} + {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $.FileNameHash $line.LeftIdx) "" -}} + + {{- $rightAnchor := Iif $line.RightIdx (printf "diff-%sR%d" $.FileNameHash $line.RightIdx) "" -}} + + {{end}} + {{$inlineDiff := $.section.GetComputedInlineDiffFor $line ctx.Locale}} {{if $inlineDiff.EscapeStatus.Escaped}}{{end}} From dbfa95bf2335d8fd634c4a537d8690975347aaae Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 11 Dec 2025 14:24:10 -0800 Subject: [PATCH 12/17] Fix format --- templates/repo/diff/section_unified.tmpl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index b4621dfa1c6a8..a5f001303fc6d 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -19,11 +19,11 @@ {{end}} {{else}} - {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $file.NameHash $line.LeftIdx) "" -}} - - {{- $rightAnchor := Iif $line.RightIdx (printf "diff-%sR%d" $file.NameHash $line.RightIdx) "" -}} - - {{end}} + {{- $leftAnchor := Iif $line.LeftIdx (printf "diff-%sL%d" $file.NameHash $line.LeftIdx) "" -}} + + {{- $rightAnchor := Iif $line.RightIdx (printf "diff-%sR%d" $file.NameHash $line.RightIdx) "" -}} + + {{end}} {{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale -}} {{- if $inlineDiff.EscapeStatus.Escaped -}} From 3140aacc51ba8a0f3532d90f9906b664465f61c6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 11 Dec 2025 15:21:01 -0800 Subject: [PATCH 13/17] Remove unnecessary code --- web_src/js/features/repo-diff.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index c36998f491a1d..f7934c724ccdf 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -25,6 +25,8 @@ function initRepoDiffFileBox(el: HTMLElement) { hideElem(queryElemSiblings(target)); showElem(target); })); + + highlightDiffSelectionFromHash(); } function initRepoDiffConversationForm() { @@ -156,8 +158,6 @@ async function onShowMoreFiles() { initViewedCheckboxListenerFor(); initImageDiff(); initDiffHeaderPopup(); - // Re-apply hash selection in case the target was just loaded - await highlightDiffSelectionFromHash(); } async function loadMoreFiles(btn: Element): Promise { From ff5544a329068885cee5950b196f0adbd4986cb8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 11 Dec 2025 15:22:44 -0800 Subject: [PATCH 14/17] Remove unnecessary code --- web_src/js/features/repo-diff.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index f7934c724ccdf..7aa0c7ca71bdc 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -25,8 +25,6 @@ function initRepoDiffFileBox(el: HTMLElement) { hideElem(queryElemSiblings(target)); showElem(target); })); - - highlightDiffSelectionFromHash(); } function initRepoDiffConversationForm() { From a9d887a5e8bab1e44f06d67d776ea5ba058d7ef7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Dec 2025 12:16:48 -0800 Subject: [PATCH 15/17] improvements --- web_src/js/features/repo-diff.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 7aa0c7ca71bdc..206b5964a7bf4 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -256,11 +256,9 @@ async function onLocationHashChange() { // eslint-disable-next-line unicorn/prefer-query-selector const commentElement = document.getElementById(hashValue); if (commentElement) { - commentElement.scrollIntoView(); - document.body.setAttribute(diffAutoScrollAttr, 'true'); + commentElement.scrollIntoView({behavior: 'instant'}); window.location.hash = ''; window.location.hash = currentHash; - setTimeout(() => document.body.removeAttribute(diffAutoScrollAttr), 0); return; } } From cf809d3dd7c213061b73551c4a5141f7d81e6e12 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Dec 2025 13:26:50 -0800 Subject: [PATCH 16/17] Fix bug --- web_src/js/features/repo-diff-selection.ts | 6 ------ web_src/js/features/repo-diff.ts | 4 +++- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/web_src/js/features/repo-diff-selection.ts b/web_src/js/features/repo-diff-selection.ts index 04ea1f9b20fb9..60fadc86e289e 100644 --- a/web_src/js/features/repo-diff-selection.ts +++ b/web_src/js/features/repo-diff-selection.ts @@ -252,12 +252,6 @@ function handleDiffLineNumberClick(cell: HTMLElement, e: MouseEvent) { export function initDiffLineSelection() { addDelegatedEventListener(document, 'click', diffLineNumberCellSelector, (cell, e) => { if (e.defaultPrevented) return; - // Ignore clicks on or inside code-expander-buttons - const target = e.target as HTMLElement; - if (target.closest('.code-expander-button') || target.closest('.code-expander-buttons') || - target.closest('button, a, input, select, textarea, summary, [role="button"]')) { - return; - } handleDiffLineNumberClick(cell, e); }); window.addEventListener('hashchange', () => { diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 206b5964a7bf4..af3ee2ba94a8a 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -170,9 +170,11 @@ async function loadMoreFiles(btn: Element): Promise { const resp = await response.text(); const respDoc = parseDom(resp, 'text/html'); const respFileBoxes = respDoc.querySelector('#diff-file-boxes')!; + const respFileBoxesChildren = Array.from(respFileBoxes.children); // respFileBoxes.children will be empty after replaceWith // the response is a full HTML page, we need to extract the relevant contents: // * append the newly loaded file list items to the existing list - document.querySelector('#diff-incomplete')!.replaceWith(...Array.from(respFileBoxes.children)); + document.querySelector('#diff-incomplete')!.replaceWith(...respFileBoxesChildren); + for (const el of respFileBoxesChildren) window.htmx.process(el); onShowMoreFiles(); return true; } catch (error) { From 4f65ae332a58637a7144375dba61afdd765d662a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Dec 2025 13:35:40 -0800 Subject: [PATCH 17/17] Add unit test for applyDiffLineSelection --- .../js/features/repo-diff-selection.test.ts | 86 +++++++++++++++++++ web_src/js/features/repo-diff-selection.ts | 2 +- 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 web_src/js/features/repo-diff-selection.test.ts diff --git a/web_src/js/features/repo-diff-selection.test.ts b/web_src/js/features/repo-diff-selection.test.ts new file mode 100644 index 0000000000000..c2b035e978efe --- /dev/null +++ b/web_src/js/features/repo-diff-selection.test.ts @@ -0,0 +1,86 @@ +import {applyDiffLineSelection} from './repo-diff-selection.ts'; + +function createDiffRow(tbody: HTMLTableSectionElement, options: {id?: string, lineType?: string} = {}) { + const tr = document.createElement('tr'); + if (options.lineType) tr.setAttribute('data-line-type', options.lineType); + + const numberCell = document.createElement('td'); + numberCell.classList.add('lines-num'); + const span = document.createElement('span'); + if (options.id) span.id = options.id; + numberCell.append(span); + tr.append(numberCell); + + tr.append(document.createElement('td')); + tbody.append(tr); + return tr; +} + +describe('applyDiffLineSelection', () => { + beforeEach(() => { + document.body.innerHTML = ''; + }); + + test('selects contiguous diff rows, skips expansion rows, and clears previous selection', () => { + const fragment = 'diff-selection'; + + const otherBox = document.createElement('div'); + const otherTable = document.createElement('table'); + otherTable.classList.add('code-diff'); + const otherTbody = document.createElement('tbody'); + const staleActiveRow = document.createElement('tr'); + staleActiveRow.classList.add('active'); + otherTbody.append(staleActiveRow); + otherTable.append(otherTbody); + otherBox.append(otherTable); + + const container = document.createElement('div'); + container.classList.add('diff-file-box'); + const table = document.createElement('table'); + table.classList.add('code-diff'); + const tbody = document.createElement('tbody'); + table.append(tbody); + container.append(table); + + const rows = [ + createDiffRow(tbody, {id: `${fragment}L1`}), + createDiffRow(tbody), + createDiffRow(tbody, {lineType: '4'}), + createDiffRow(tbody), + createDiffRow(tbody, {id: `${fragment}R5`}), + createDiffRow(tbody), + ]; + + document.body.append(otherBox, container); + + const range = {fragment, startSide: 'L' as const, startLine: 1, endSide: 'R' as const, endLine: 5}; + const applied = applyDiffLineSelection(container, range); + + expect(applied).toBe(true); + expect(rows[0].classList.contains('active')).toBe(true); + expect(rows[1].classList.contains('active')).toBe(true); + expect(rows[2].classList.contains('active')).toBe(false); + expect(rows[3].classList.contains('active')).toBe(true); + expect(rows[4].classList.contains('active')).toBe(true); + expect(rows[5].classList.contains('active')).toBe(false); + expect(staleActiveRow.classList.contains('active')).toBe(false); + }); + + test('returns false when either anchor is missing', () => { + const fragment = 'diff-missing'; + const container = document.createElement('div'); + container.classList.add('diff-file-box'); + const table = document.createElement('table'); + table.classList.add('code-diff'); + const tbody = document.createElement('tbody'); + table.append(tbody); + container.append(table); + document.body.append(container); + + createDiffRow(tbody, {id: `${fragment}L1`}); + + const applied = applyDiffLineSelection(container, {fragment, startSide: 'L' as const, startLine: 1, endSide: 'R' as const, endLine: 2}); + expect(applied).toBe(false); + expect(container.querySelectorAll('tr.active').length).toBe(0); + }); +}); diff --git a/web_src/js/features/repo-diff-selection.ts b/web_src/js/features/repo-diff-selection.ts index 60fadc86e289e..bc00f04fdfbea 100644 --- a/web_src/js/features/repo-diff-selection.ts +++ b/web_src/js/features/repo-diff-selection.ts @@ -36,7 +36,7 @@ function parseDiffAnchor(anchor: string): DiffAnchorInfo | null { return {anchor, fragment, side, line}; } -function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRange): boolean { +export function applyDiffLineSelection(container: HTMLElement, range: DiffSelectionRange): boolean { // Find the start and end anchor elements const startId = `${range.fragment}${range.startSide}${range.startLine}`; const endId = `${range.fragment}${range.endSide}${range.endLine}`;