Skip to content

fix: improve hash parsing in file renderers #3825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 40 additions & 19 deletions src/ui/VirtualRenderers/VirtualDiffRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,26 +129,47 @@ const CodeBody = ({
codeDisplayOverlayRef.current.style.height = `${virtualizer.getTotalSize()}px`
codeDisplayOverlayRef.current.style.position = 'relative'

const lineHash = location.hash.split('-')?.[0]?.slice(1)
if (lineHash === hashedPath) {
const lineIndicator = location.hash.split('-')?.[1]
const isBase = lineIndicator?.includes('L')
const isHead = lineIndicator?.includes('R')
const hashLineNumber = lineIndicator?.slice(1)

const index = lineData.findIndex(
(line) =>
(isHead && line.headNumber === hashLineNumber) ||
(isBase && line.baseNumber === hashLineNumber)
)

if (index >= 0 && index < tokens.length) {
virtualizer.scrollToIndex(index, { align: 'start' })
// More robust hash parsing to handle different formats
if (location.hash) {
// First try to parse the complex format: #path-[LR]number
const complexMatch = location.hash.match(/^#(.*)-([LR])(\d+)$/)
if (complexMatch && complexMatch.length === 4) {
const [, path, lineType, lineNumberStr] = complexMatch

if (path === hashedPath) {
const lineNumber = lineNumberStr
const isBase = lineType === 'L'
const isHead = lineType === 'R'

const index = lineData.findIndex(
(line) =>
(isHead && line.headNumber === lineNumber) ||
(isBase && line.baseNumber === lineNumber)
)

if (index >= 0 && index < tokens.length) {
virtualizer.scrollToIndex(index, { align: 'start' })
} else {
Sentry.captureMessage(
`Invalid line number in file renderer hash: ${location.hash}`,
{ fingerprint: ['file-renderer-invalid-line-number'] }
)
}
}
} else {
Sentry.captureMessage(
`Invalid line number in file renderer hash: ${location.hash}`,
{ fingerprint: ['file-renderer-invalid-line-number'] }
)
// Try to handle simple format: #Lnumber
const simpleMatch = location.hash.match(/^#L(\d+)$/)
if (simpleMatch && simpleMatch[1]) {
const lineNumber = simpleMatch[1]
// For simple format, try to find a matching base line number
const index = lineData.findIndex(
(line) => line.baseNumber === lineNumber
)

if (index >= 0 && index < tokens.length) {
virtualizer.scrollToIndex(index, { align: 'start' })
}
}
}
}
}, [
Expand Down
24 changes: 19 additions & 5 deletions src/ui/VirtualRenderers/VirtualFileRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,26 @@ const CodeBody = ({
codeDisplayOverlayRef.current.style.height = `${virtualizer.getTotalSize()}px`
codeDisplayOverlayRef.current.style.position = 'relative'

const index = parseInt(location.hash.slice(2), 10)
// need to check !isNaN because parseInt return NaN if the string is not a number which is still a valid number.
if (!isNaN(index) && index > 0 && index <= tokens.length) {
// More robust hash parsing to handle different formats
let lineNumber = null
if (location.hash) {
const simpleLineMatch = location.hash.match(/^#L(\d+)$/)
if (simpleLineMatch && simpleLineMatch[1]) {
lineNumber = parseInt(simpleLineMatch[1], 10)
} else {
const complexLineMatch = location.hash.match(/^#.*-[LR](\d+)$/)
if (complexLineMatch && complexLineMatch[1]) {
lineNumber = parseInt(complexLineMatch[1], 10)
}
}
}

// If we found a valid line number and it's within range, scroll to it
if (lineNumber && !isNaN(lineNumber) && lineNumber > 0 && lineNumber <= tokens.length) {
// need to adjust from line number back to array index
virtualizer.scrollToIndex(index - 1, { align: 'start' })
} else {
virtualizer.scrollToIndex(lineNumber - 1, { align: 'start' })
} else if (location.hash) {
// Only log the error if there was a hash that we couldn't parse properly
Sentry.captureMessage(
`Invalid line number in file renderer hash: ${location.hash}`,
{ fingerprint: ['file-renderer-invalid-line-number'] }
Expand Down
Loading