diff --git a/CHANGELOG.md b/CHANGELOG.md index a2db78d7f..44c52d5e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Add missing schema changes introduced in [#1170](https://github.com/sourcebot-dev/sourcebot/pull/1170). [#1176](https://github.com/sourcebot-dev/sourcebot/pull/1176) +- Fixed blame gutter commit navigation to use the file path as it existed at the attributing commit, so clicking a blame line whose commit predates a rename resolves to the correct historical path. [#1178](https://github.com/sourcebot-dev/sourcebot/pull/1178) ## [4.17.1] - 2026-05-04 diff --git a/docs/api-reference/sourcebot-public.openapi.json b/docs/api-reference/sourcebot-public.openapi.json index 96c996b20..869ee2bfd 100644 --- a/docs/api-reference/sourcebot-public.openapi.json +++ b/docs/api-reference/sourcebot-public.openapi.json @@ -599,6 +599,10 @@ "type": "string", "description": "The hash of the commit that last modified the lines in this range." }, + "path": { + "type": "string", + "description": "The file path as it existed at the attributing commit. May differ from the current path due to renames." + }, "startLine": { "type": "integer", "minimum": 0, @@ -614,6 +618,7 @@ }, "required": [ "hash", + "path", "startLine", "lineCount" ] diff --git a/packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/blameGutterExtension.ts b/packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/blameGutterExtension.ts index 3cf74f268..423ff1e70 100644 --- a/packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/blameGutterExtension.ts +++ b/packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/blameGutterExtension.ts @@ -7,6 +7,10 @@ import { BLAME_AGE_BG_CLASSES, computeAgeBucket } from "./blameAgeColors"; type LineEntry = { hash: string; + // The file path as it existed at `hash`. May differ from the current file + // path if the file was renamed in or after that commit, so we use it + // (rather than the current path) when navigating to the commit's diff. + path: string; // Set only on the first line of a contiguous range; null on continuation // lines so they render as empty filler cells. message: string | null; @@ -29,7 +33,7 @@ const FILE_STACK_SVG = ' void, + onCommitClick: (commit: { hash: string; path: string }) => void, onReblameClick: (previous: { hash: string; path: string }) => void, ): HTMLElement => { const cell = document.createElement('div'); @@ -80,7 +84,7 @@ const buildCellDom = ( messageEl.type = 'button'; messageEl.className = 'flex-1 min-w-0 truncate text-left bg-transparent border-0 p-0 m-0 font-[inherit] text-inherit cursor-pointer hover:text-foreground hover:underline'; messageEl.textContent = entry.message; - messageEl.addEventListener('click', () => onCommitClick(entry.hash)); + messageEl.addEventListener('click', () => onCommitClick({ hash: entry.hash, path: entry.path })); cell.appendChild(messageEl); if (entry.previous) { @@ -103,7 +107,7 @@ const buildCellDom = ( class BlameMarker extends GutterMarker { constructor( readonly entry: LineEntry, - readonly onCommitClick: (hash: string) => void, + readonly onCommitClick: (commit: { hash: string; path: string }) => void, readonly onReblameClick: (previous: { hash: string; path: string }) => void, ) { super(); @@ -117,6 +121,7 @@ class BlameMarker extends GutterMarker { const b = other.entry; return ( a.hash === b.hash && + a.path === b.path && a.message === b.message && a.date === b.date && a.authorEmail === b.authorEmail && @@ -199,6 +204,7 @@ const buildLineIndex = (blame: FileBlameResponse): Map => { if (isFirstLineOfRange && commit) { index.set(lineNumber, { hash: range.hash, + path: range.path, message: commit.message, date: commit.date, authorEmail: commit.authorEmail, @@ -209,6 +215,7 @@ const buildLineIndex = (blame: FileBlameResponse): Map => { } else { index.set(lineNumber, { hash: range.hash, + path: range.path, message: null, date: null, authorEmail: null, @@ -236,7 +243,7 @@ const blameTheme = EditorView.theme({ export const blameGutterExtension = ( blame: FileBlameResponse, - onCommitClick: (hash: string) => void, + onCommitClick: (commit: { hash: string; path: string }) => void, onReblameClick: (previous: { hash: string; path: string }) => void, ): Extension => { const lineIndex = buildLineIndex(blame); diff --git a/packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsx b/packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsx index cbd14eeba..66af078f3 100644 --- a/packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsx +++ b/packages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsx @@ -42,16 +42,16 @@ export const PureCodePreviewPanel = ({ const hasCodeNavEntitlement = useHasEntitlement("code-nav"); const router = useRouter(); - const handleBlameCommitClick = useCallback((hash: string) => { + const handleBlameCommitClick = useCallback((commit: { hash: string; path: string }) => { router.push(getBrowsePath({ repoName, revisionName, - path, + path: commit.path, pathType: 'blob', - previewRef: hash, + previewRef: commit.hash, diff: true, })); - }, [router, repoName, revisionName, path]); + }, [router, repoName, revisionName]); const handleBlameReblameClick = useCallback((previous: { hash: string; path: string }) => { router.push(getBrowsePath({ diff --git a/packages/web/src/features/git/getFileBlameApi.ts b/packages/web/src/features/git/getFileBlameApi.ts index a7a63006a..15c0d684d 100644 --- a/packages/web/src/features/git/getFileBlameApi.ts +++ b/packages/web/src/features/git/getFileBlameApi.ts @@ -21,12 +21,12 @@ type CommitMeta = FileBlameResponse['commits'][string]; * Format reference: each blamed line produces an entry of the form * * [] (4-field header → first line of a group) - * [author (metadata block, only on first - * author-mail <> appearance of a commit globally) - * author-time - * author-tz <+/-HHMM> - * committer ... - * summary + * [author (metadata block, emitted only on + * author-mail <> the first global appearance of a + * author-time commit; subsequent groups for the + * author-tz <+/-HHMM> same commit are header-only. With + * committer ... -C/-M, `filename` may be re-emitted + * summary if it differs from the prior value.) * previous (optional) * filename ] * \t @@ -34,10 +34,14 @@ type CommitMeta = FileBlameResponse['commits'][string]; * Within a contiguous group of lines from the same commit, only the first line's * header carries ``; subsequent lines have a 3-field header. We detect * group boundaries via the presence of `` and emit one range per group. + * + * Because `filename` is emitted per-commit (not per-group), we cache it in + * `filenameByHash` and look it up when pushing a range. */ const parsePorcelainBlame = (output: string): FileBlameResponse => { const ranges: FileBlameResponse['ranges'] = []; const commits: Record = {}; + const filenameByHash = new Map(); if (output.length === 0) { return { ranges, commits }; @@ -102,8 +106,10 @@ const parsePorcelainBlame = (output: string): FileBlameResponse => { path: value.substring(sep + 1), }; } + } else if (key === 'filename') { + filenameByHash.set(hash, value); } - // committer*, filename, boundary are intentionally ignored. + // committer*, boundary are intentionally ignored. i++; } @@ -125,7 +131,11 @@ const parsePorcelainBlame = (output: string): FileBlameResponse => { } if (isGroupStart) { - ranges.push({ hash, startLine: finalLine, lineCount }); + const path = filenameByHash.get(hash); + if (path === undefined) { + throw new Error(`Malformed git blame porcelain output: missing "filename" for commit ${hash}`); + } + ranges.push({ hash, path, startLine: finalLine, lineCount }); } } @@ -136,7 +146,7 @@ const parsePorcelainBlame = (output: string): FileBlameResponse => { const coalescedRanges: FileBlameResponse['ranges'] = []; for (const range of ranges) { const last = coalescedRanges[coalescedRanges.length - 1]; - if (last && last.hash === range.hash && last.startLine + last.lineCount === range.startLine) { + if (last && last.hash === range.hash && last.path === range.path && last.startLine + last.lineCount === range.startLine) { last.lineCount += range.lineCount; } else { coalescedRanges.push({ ...range }); diff --git a/packages/web/src/features/git/schemas.ts b/packages/web/src/features/git/schemas.ts index de1ed9ec3..a73e45c0b 100644 --- a/packages/web/src/features/git/schemas.ts +++ b/packages/web/src/features/git/schemas.ts @@ -119,6 +119,7 @@ export const fileBlameRequestSchema = z.object({ export const blameRangeSchema = z.object({ hash: z.string().describe('The hash of the commit that last modified the lines in this range.'), + path: z.string().describe('The file path as it existed at the attributing commit. May differ from the current path due to renames.'), startLine: z.number().int().positive().describe('The 1-based line number where the range begins (inclusive).'), lineCount: z.number().int().positive().describe('The number of contiguous lines in this range.'), });