[wip] Commit diffs#1154
Open
brendan-kellam wants to merge 10 commits intomainfrom
Open
Conversation
Contributor
|
@brendan-kellam your pull request is missing a changelog! |
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (36)
WalkthroughThis PR introduces a commit diff viewing feature with dedicated components for displaying commit details and file-level diffs, adds shared code highlighting utilities, refactors imports to absolute paths throughout the browse module, extracts git logging, and adds a debug flag for React GRAP in development environments. Changes
Sequence DiagramsequenceDiagram
participant User
participant BrowsePage
participant Router
participant CommitDiffPanel
participant DiffAPI
participant FileDiffList
participant LightweightDiffViewer
participant CodeHighlighter
User->>Router: Navigate to /-/commit/abc123/path
Router->>BrowsePage: Match commit route with pathType='commit'
BrowsePage->>CommitDiffPanel: Render with commitSha
CommitDiffPanel->>DiffAPI: getCommit(commitSha)
CommitDiffPanel->>DiffAPI: getDiff(parentSha..commitSha)
DiffAPI-->>CommitDiffPanel: Return commit & diff
CommitDiffPanel->>CommitDiffPanel: Compute stats, format metadata
CommitDiffPanel->>FileDiffList: Render with files array
FileDiffList->>FileDiffList: Virtualize file diffs
FileDiffList->>LightweightDiffViewer: Render per-file diff
LightweightDiffViewer->>CodeHighlighter: highlightCode(hunks, language)
CodeHighlighter-->>LightweightDiffViewer: Highlighted ranges with inner-diff
LightweightDiffViewer-->>FileDiffList: Rendered diff row
FileDiffList-->>CommitDiffPanel: Scrollable file diff list
CommitDiffPanel-->>BrowsePage: Full commit diff view
BrowsePage-->>User: Display commit with stats & file diffs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
License Audit
Weak Copyleft Packages (informational)
Resolved Packages (11)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
packages/web/src/app/layout.tsx (1)
47-59: Consider extracting the repeated debug predicate.Same condition is duplicated for both scripts; a shared local constant keeps this easier to maintain.
Refactor sketch
export default function RootLayout({ children, }: Readonly<{ children: React.ReactNode; }>) { + const isReactGrabDebugEnabled = + env.NODE_ENV === "development" && env.DEBUG_ENABLE_REACT_GRAP === "true"; + return ( <html @@ - {env.NODE_ENV === "development" && env.DEBUG_ENABLE_REACT_GRAP === 'true' && ( + {isReactGrabDebugEnabled && ( <Script src="//unpkg.com/react-grab/dist/index.global.js" crossOrigin="anonymous" strategy="beforeInteractive" /> )} - {env.NODE_ENV === "development" && env.DEBUG_ENABLE_REACT_GRAP === 'true' && ( + {isReactGrabDebugEnabled && ( <Script src="//unpkg.com/@react-grab/mcp/dist/client.global.js" strategy="lazyOnload" /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/layout.tsx` around lines 47 - 59, Extract the duplicated predicate into a single local constant (e.g. isDevReactGrabEnabled) inside the component in layout.tsx and replace both occurrences of env.NODE_ENV === "development" && env.DEBUG_ENABLE_REACT_GRAP === 'true' with that constant; ensure the constant is computed before the JSX and used to gate both <Script ... /> renderings so the behavior is unchanged but maintenance is easier.packages/web/src/lib/codeHighlight.ts (1)
38-50: Cache parser lookup to avoid per-line resolution overhead.
highlightCoderesolves parser every call (Line 50), and it’s invoked per line inpackages/web/src/app/(app)/components/lightweightCodeHighlighter.tsx(Line 60-77). A small cache avoids repeated language matching/loading work.Proposed refactor
+const parserByLanguageNameCache = new Map<string, Promise<Parser>>(); + export const getCodeParserByLanguageName = async (languageName: string): Promise<Parser> => { - if (!languageName) { - return plainTextLanguage.parser; - } - const found = LanguageDescription.matchLanguageName(builtinLanguages, languageName, true); - if (!found) { - return plainTextLanguage.parser; - } - if (!found.support) { - try { - await found.load(); - } catch { - return plainTextLanguage.parser; - } - } - return found.support ? found.support.language.parser : plainTextLanguage.parser; + const key = languageName?.toLowerCase() ?? ''; + if (!parserByLanguageNameCache.has(key)) { + parserByLanguageNameCache.set( + key, + (async () => { + if (!languageName) { + return plainTextLanguage.parser; + } + const found = LanguageDescription.matchLanguageName(builtinLanguages, languageName, true); + if (!found) { + return plainTextLanguage.parser; + } + if (!found.support) { + try { + await found.load(); + } catch { + return plainTextLanguage.parser; + } + } + return found.support ? found.support.language.parser : plainTextLanguage.parser; + })(), + ); + } + return parserByLanguageNameCache.get(key)!; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/codeHighlight.ts` around lines 38 - 50, highlightCode currently calls getCodeParserByLanguageName on every invocation (causing heavy per-line overhead when invoked from lightweightCodeHighlighter), so introduce a simple module-level cache (e.g., a Map<string, Parser>) and use it inside highlightCode to return a cached parser if present; if missing, call getCodeParserByLanguageName, store the result in the cache, then proceed as before. Update references to highlightCode and ensure the cache key is the languageName string and the behavior unchanged for callers such as lightweightCodeHighlighter; no external API changes required.packages/web/src/app/(app)/components/pathHeader.tsx (1)
118-118: Consider replacing the hard-coded width reserve with a named constant.
40works, but this is easy to drift as button/icon spacing evolves. A named constant (or measured control width) would make future tweaks safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/components/pathHeader.tsx at line 118, Replace the magic number 40 used when computing availableWidth (const availableWidth = containerWidth - 40) with a named constant (e.g., CONTROL_RESERVE_WIDTH or COPY_BUTTON_RESERVED_WIDTH) declared near the top of the PathHeader component or module; update the subtraction to use that constant and adjust the comment to explain what the reserved width covers (copy button, padding, icon spacing), or alternatively compute the reserve by measuring the control's offsetWidth if dynamic sizing is needed—this makes future spacing changes safer and easier to find.packages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx (1)
62-67: Consider adding error handling for clipboard operations.The clipboard write could fail (e.g., due to permissions or secure context requirements). Currently, errors are silently ignored.
♻️ Suggested improvement
const onCopySha = useCallback(() => { - navigator.clipboard.writeText(commit.hash).then(() => { - toast({ description: "✅ Copied commit SHA to clipboard" }); - }); + navigator.clipboard.writeText(commit.hash) + .then(() => { + toast({ description: "✅ Copied commit SHA to clipboard" }); + }) + .catch(() => { + toast({ description: "❌ Failed to copy to clipboard", variant: "destructive" }); + }); return true; }, [commit.hash, toast]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx around lines 62 - 67, The onCopySha callback currently calls navigator.clipboard.writeText(commit.hash) without handling failures; update onCopySha (the useCallback) to handle errors by awaiting or using .then/.catch on navigator.clipboard.writeText(commit.hash) and on failure show a toast error (e.g., toast({ variant: "destructive", description: "Failed to copy SHA" })) and optionally log the error; ensure you still return a boolean and keep dependencies [commit.hash, toast] unchanged.packages/web/src/app/(app)/browse/[...path]/page.tsx (1)
116-145: Consider extracting the panel selection into a helper or switch statement.The nested ternary chain for selecting which panel to render is functional but could become harder to maintain as more path types are added. A switch statement or mapping object would improve readability.
♻️ Optional refactor using a switch-like pattern
const renderPanel = () => { switch (browseProps.pathType) { case 'blob': return <CodePreviewPanel path={path} repoName={repoName} revisionName={revisionName} />; case 'commits': return <CommitsPanel path={path} repoName={repoName} revisionName={revisionName} page={page} author={author} since={since} until={until} />; case 'commit': return <CommitDiffPanel repoName={repoName} revisionName={revisionName} commitSha={browseProps.commitSha} path={path} />; case 'tree': return <TreePreviewPanel path={path} repoName={repoName} revisionName={revisionName} />; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/page.tsx around lines 116 - 145, Extract the nested ternary into a small helper (e.g., renderPanel) that switches on browseProps.pathType and returns the appropriate panel component; replace the current inline ternary with a single call to renderPanel() and ensure the helper returns CodePreviewPanel, CommitsPanel (passing page, author, since, until), CommitDiffPanel (using browseProps.commitSha), or TreePreviewPanel as appropriate to keep prop passing identical.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsx (1)
18-23: Same clipboard error handling suggestion applies here.This has the same pattern as
commitRow.tsx- consider adding error handling for the clipboard operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsx around lines 18 - 23, The onCopyHash handler in commitHashLine.tsx calls navigator.clipboard.writeText(commitHash) without handling failures; update the onCopyHash function (the useCallback named onCopyHash) to handle rejected writes: either make it async and await writeText inside a try/catch or append a .catch handler, and on error call toast with an error description (and optionally log the error) while still returning an appropriate boolean; ensure you reference navigator.clipboard.writeText, commitHash, and toast in the new error path so users get feedback when the copy fails.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx (1)
27-27: UnusedrevisionNameprop.The
revisionNameparameter is declared in the props interface but never used in the component. Either remove it from the interface or use it if it's needed for future functionality.♻️ If unused, remove from props
-export const CommitDiffPanel = async ({ repoName, commitSha, path }: CommitDiffPanelProps) => { +export const CommitDiffPanel = async ({ repoName, revisionName, commitSha, path }: CommitDiffPanelProps) => {Or if truly not needed:
interface CommitDiffPanelProps { repoName: string; - revisionName?: string; commitSha: string; path: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx at line 27, The props interface includes an unused property revisionName referenced by CommitDiffPanelProps while CommitDiffPanel does not use it; remove revisionName from the CommitDiffPanelProps interface (or, if intended, add usage of revisionName inside the CommitDiffPanel component where repoName/commitSha/path are used) so the prop is either consumed or eliminated — update any call sites that pass revisionName accordingly and run type checks to ensure no remaining references.packages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx (1)
100-107: Redundantkeyprop onFileDiffRow.The
key={rowKey}onFileDiffRowis unnecessary since the parentdivalready haskey={virtualRow.key}. React only needs keys on the direct children of the mapped array.♻️ Remove redundant key
<FileDiffRow - key={rowKey} file={file} yOffset={virtualRow.start} repoName={repoName} commitSha={commitSha} parentSha={parentSha} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx around lines 100 - 107, The FileDiffRow instance is receiving a redundant key prop (key={rowKey}) even though its parent element uses key={virtualRow.key}; remove the unnecessary key prop from the FileDiffRow JSX in fileDiffList.tsx so only the parent wrapper provides the key, leaving props file, yOffset, repoName, commitSha and parentSha unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/lib/codeHighlight.ts`:
- Around line 21-24: The code calls found.load() without handling rejections
which can break highlighting; update the logic in codeHighlight.ts so the await
found.load() is wrapped with error handling (try/catch or .catch) and if loading
fails or found.support is still falsy return plainTextLanguage.parser as a safe
fallback; apply the same change to the second occurrence around lines 32–35 (the
other branch that accesses found.support and found.load()) and reference
found.load(), found.support and plainTextLanguage.parser when making the change.
In `@packages/web/src/lib/utils.ts`:
- Around line 31-35: The regex currently truncates any string starting with 40
hex chars; change it to only treat a ref as a SHA when it is exactly 40 hex
chars or when it is followed by a non-alphanumeric separator. Replace the match
logic using ref.match(/^([0-9a-f]{40})(.*)$/i) with a regex that makes the
suffix optional and only captures it if it begins with a non-alphanumeric
character (for example /^([0-9a-f]{40})([^0-9A-Za-z].*)?$/i), then return
match[1].slice(0,7) + (match[2] || '') so symbolic refs that start with 40 hex
chars but have an alphanumeric suffix are not truncated.
---
Nitpick comments:
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsx:
- Line 27: The props interface includes an unused property revisionName
referenced by CommitDiffPanelProps while CommitDiffPanel does not use it; remove
revisionName from the CommitDiffPanelProps interface (or, if intended, add usage
of revisionName inside the CommitDiffPanel component where
repoName/commitSha/path are used) so the prop is either consumed or eliminated —
update any call sites that pass revisionName accordingly and run type checks to
ensure no remaining references.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsx:
- Around line 18-23: The onCopyHash handler in commitHashLine.tsx calls
navigator.clipboard.writeText(commitHash) without handling failures; update the
onCopyHash function (the useCallback named onCopyHash) to handle rejected
writes: either make it async and await writeText inside a try/catch or append a
.catch handler, and on error call toast with an error description (and
optionally log the error) while still returning an appropriate boolean; ensure
you reference navigator.clipboard.writeText, commitHash, and toast in the new
error path so users get feedback when the copy fails.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsx:
- Around line 100-107: The FileDiffRow instance is receiving a redundant key
prop (key={rowKey}) even though its parent element uses key={virtualRow.key};
remove the unnecessary key prop from the FileDiffRow JSX in fileDiffList.tsx so
only the parent wrapper provides the key, leaving props file, yOffset, repoName,
commitSha and parentSha unchanged.
In
`@packages/web/src/app/`(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsx:
- Around line 62-67: The onCopySha callback currently calls
navigator.clipboard.writeText(commit.hash) without handling failures; update
onCopySha (the useCallback) to handle errors by awaiting or using .then/.catch
on navigator.clipboard.writeText(commit.hash) and on failure show a toast error
(e.g., toast({ variant: "destructive", description: "Failed to copy SHA" })) and
optionally log the error; ensure you still return a boolean and keep
dependencies [commit.hash, toast] unchanged.
In `@packages/web/src/app/`(app)/browse/[...path]/page.tsx:
- Around line 116-145: Extract the nested ternary into a small helper (e.g.,
renderPanel) that switches on browseProps.pathType and returns the appropriate
panel component; replace the current inline ternary with a single call to
renderPanel() and ensure the helper returns CodePreviewPanel, CommitsPanel
(passing page, author, since, until), CommitDiffPanel (using
browseProps.commitSha), or TreePreviewPanel as appropriate to keep prop passing
identical.
In `@packages/web/src/app/`(app)/components/pathHeader.tsx:
- Line 118: Replace the magic number 40 used when computing availableWidth
(const availableWidth = containerWidth - 40) with a named constant (e.g.,
CONTROL_RESERVE_WIDTH or COPY_BUTTON_RESERVED_WIDTH) declared near the top of
the PathHeader component or module; update the subtraction to use that constant
and adjust the comment to explain what the reserved width covers (copy button,
padding, icon spacing), or alternatively compute the reserve by measuring the
control's offsetWidth if dynamic sizing is needed—this makes future spacing
changes safer and easier to find.
In `@packages/web/src/app/layout.tsx`:
- Around line 47-59: Extract the duplicated predicate into a single local
constant (e.g. isDevReactGrabEnabled) inside the component in layout.tsx and
replace both occurrences of env.NODE_ENV === "development" &&
env.DEBUG_ENABLE_REACT_GRAP === 'true' with that constant; ensure the constant
is computed before the JSX and used to gate both <Script ... /> renderings so
the behavior is unchanged but maintenance is easier.
In `@packages/web/src/lib/codeHighlight.ts`:
- Around line 38-50: highlightCode currently calls getCodeParserByLanguageName
on every invocation (causing heavy per-line overhead when invoked from
lightweightCodeHighlighter), so introduce a simple module-level cache (e.g., a
Map<string, Parser>) and use it inside highlightCode to return a cached parser
if present; if missing, call getCodeParserByLanguageName, store the result in
the cache, then proceed as before. Update references to highlightCode and ensure
the cache key is the languageName string and the behavior unchanged for callers
such as lightweightCodeHighlighter; no external API changes required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 542c5684-421f-4dc6-8132-95d09095e896
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
packages/shared/src/env.server.tspackages/web/package.jsonpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/codePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/pureCodePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/codePreviewPanel/rangeHighlightingExtension.tspackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitDiffPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitHashLine.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/commitMessage.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/diffStat.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffList.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/fileDiffRow.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/hunkParser.tspackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/lightweightDiffViewer.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitDiffPanel/splitPairing.tspackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/authorFilter.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitRow.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPagination.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/commitsPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/commitHistoryPanel/dateFilter.tsxpackages/web/src/app/(app)/browse/[...path]/components/treePreviewPanel/pureTreePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/components/treePreviewPanel/treePreviewPanel.tsxpackages/web/src/app/(app)/browse/[...path]/page.tsxpackages/web/src/app/(app)/browse/components/historyRow.tsxpackages/web/src/app/(app)/browse/hooks/useBrowseNavigation.tspackages/web/src/app/(app)/browse/hooks/utils.tspackages/web/src/app/(app)/components/lightweightCodeHighlighter.tsxpackages/web/src/app/(app)/components/pathHeader.tsxpackages/web/src/app/layout.tsxpackages/web/src/features/chat/components/chatThread/tools/getDiffToolComponent.tsxpackages/web/src/features/git/getFilesApi.tspackages/web/src/features/git/getFolderContentsApi.tspackages/web/src/features/git/getTreeApi.tspackages/web/src/features/git/logger.tspackages/web/src/features/git/utils.tspackages/web/src/lib/codeHighlight.tspackages/web/src/lib/utils.ts
💤 Files with no reviewable changes (1)
- packages/web/src/features/git/utils.ts
Comment on lines
+21
to
+24
| if (!found.support) { | ||
| await found.load(); | ||
| } | ||
| return found.support ? found.support.language.parser : plainTextLanguage.parser; |
Contributor
There was a problem hiding this comment.
Handle language-module load failures with a safe fallback.
If found.load() rejects, highlighting fails for the whole call path. Falling back to plainTextLanguage.parser preserves rendering instead of failing hard.
Proposed fix
export const getCodeParserByLanguageName = async (languageName: string): Promise<Parser> => {
@@
- if (!found.support) {
- await found.load();
- }
+ if (!found.support) {
+ try {
+ await found.load();
+ } catch {
+ return plainTextLanguage.parser;
+ }
+ }
return found.support ? found.support.language.parser : plainTextLanguage.parser;
};
@@
export const getCodeParserByFilename = async (filename: string): Promise<Parser> => {
@@
- if (!found.support) {
- await found.load();
- }
+ if (!found.support) {
+ try {
+ await found.load();
+ } catch {
+ return plainTextLanguage.parser;
+ }
+ }
return found.support ? found.support.language.parser : plainTextLanguage.parser;
};Also applies to: 32-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/lib/codeHighlight.ts` around lines 21 - 24, The code calls
found.load() without handling rejections which can break highlighting; update
the logic in codeHighlight.ts so the await found.load() is wrapped with error
handling (try/catch or .catch) and if loading fails or found.support is still
falsy return plainTextLanguage.parser as a safe fallback; apply the same change
to the second occurrence around lines 32–35 (the other branch that accesses
found.support and found.load()) and reference found.load(), found.support and
plainTextLanguage.parser when making the change.
Comment on lines
+31
to
+35
| const match = ref.match(/^([0-9a-f]{40})(.*)$/i); | ||
| if (match) { | ||
| return match[1].slice(0, 7) + match[2]; | ||
| } | ||
| return ref; |
Contributor
There was a problem hiding this comment.
Restrict SHA matching to true SHA refs to avoid truncating symbolic refs.
The current regex truncates any string that starts with 40 hex chars, even if it’s actually a symbolic ref with a non-operator suffix. That conflicts with the function contract.
Suggested fix
export function truncateSha(ref: string): string {
- const match = ref.match(/^([0-9a-f]{40})(.*)$/i);
+ const match = ref.match(/^([0-9a-f]{40})([~^].*)?$/i);
if (match) {
- return match[1].slice(0, 7) + match[2];
+ return match[1].slice(0, 7) + (match[2] ?? "");
}
return ref;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const match = ref.match(/^([0-9a-f]{40})(.*)$/i); | |
| if (match) { | |
| return match[1].slice(0, 7) + match[2]; | |
| } | |
| return ref; | |
| export function truncateSha(ref: string): string { | |
| const match = ref.match(/^([0-9a-f]{40})([~^].*)?$/i); | |
| if (match) { | |
| return match[1].slice(0, 7) + (match[2] ?? ""); | |
| } | |
| return ref; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/lib/utils.ts` around lines 31 - 35, The regex currently
truncates any string starting with 40 hex chars; change it to only treat a ref
as a SHA when it is exactly 40 hex chars or when it is followed by a
non-alphanumeric separator. Replace the match logic using
ref.match(/^([0-9a-f]{40})(.*)$/i) with a regex that makes the suffix optional
and only captures it if it begins with a non-alphanumeric character (for example
/^([0-9a-f]{40})([^0-9A-Za-z].*)?$/i), then return match[1].slice(0,7) +
(match[2] || '') so symbolic refs that start with 40 hex chars but have an
alphanumeric suffix are not truncated.
Adds a `commit` pathType to the browse routes (`/browse/<repo>@<branch>/-/commit/<sha>[/<file>]`) that renders a placeholder CommitDiffPanel. Refactors browse path helpers into a discriminated `BrowseProps` union so commitSha is required only for pathType: 'commit'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires up @codemirror/merge (via react-codemirror-merge) inside CommitDiffPanel with a static before/after demo. Adds a CodeDiff component that owns its language extension + view ref so each pane can reconfigure its language compartment independently. Also gates the react-grab dev scripts behind DEBUG_ENABLE_REACT_GRAP so they don't load on every dev page render. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the CodeMirror MergeView-based commit diff rendering with a DOM based split-view that renders directly from git's hunks, inspired by Chrome DevTools' DiffView. Per-file editor instances and the matching getFileSource fetches are gone — a 50-file commit drops from ~100 network requests to 0, and per-row render cost from a full editor mount to a synchronous Lezer highlight + grid emit. - New `LightweightDiffViewer` builds a single 2-column subgrid with hunk headers spanning both sides; each cell uses `subgrid` so line numbers, markers, and content align across all rows. - Pure helpers split out: `hunkParser` (body string → DiffLine[]), `splitPairing` (DiffLine[] → SplitRow[]). - `presentableDiff` from @codemirror/merge supplies character-level intra-line diff highlighting on paired modifications. - Lezer highlight code lifted from `lightweightCodeHighlighter` into `lib/codeHighlight` so both files share the helper. - Drop `react-codemirror-merge` and `commitDiffEditor`. Long lines wrap via `whitespace-pre-wrap break-words` + `minmax(0, 1fr)` on the grid. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- File path header now sticks to the top of the scroll viewport while scrolling through that file's diff, using the negative-yOffset trick to compensate for the virtualizer's translateY positioning. Same pattern as searchResultsPanel/fileMatchContainer. - Lightweight diff viewer's grid uses `minmax(<min>, max-content)` for line-number and marker columns so they don't collapse to zero width when one side of the diff is entirely blank (fully-added or fully-deleted files), keeping the right pane aligned across files. - Drop the column gap between left and right panes and instead draw a `border-r` separator on the left cells for a cleaner divider. - Hunk header gets an optional className so the first hunk renders with just `border-b` (the file header above already provides the top border), while subsequent hunks render with `border-y` between them. - Drop the per-row footer padding in the virtualizer; rows now sit flush against each other. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- New `DiffStat` component renders GitHub-style additions/deletions counts with a 5-square indicator scaled log-ish to total change size. Added on the right of each file row header and on the right of the "N files changed" subheader for the commit total. Hidden when there are no line-level changes (pure renames). - Each file row gets a `CopyIconButton` next to the path (copies newPath, falling back to oldPath) and a `CommitActionLink` that opens the file at the commit. Deleted files link to the old path at the parent commit so the user lands on the file's last existing state rather than a 404. - `repoName`, `commitSha`, and `parentSha` are plumbed from the panel through `FileDiffList` to `FileDiffRow` to support the new link. - `computeChangeCounts` is memoized per file in the row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nchoring
History panel rows in both the bottom panel and the commits page are now
clickable — they navigate to the matching commit diff via router.push,
with closest('button, a') short-circuit so inner action buttons keep
their own behavior. Bottom-panel history rows also highlight via
bg-accent when their commit is the one currently being viewed.
Commit diff header now uses AuthorsAvatarGroup + getCommitAuthors +
formatAuthorsText, matching latestCommitInfo and historyRow — co-authors
parsed from the commit body show up correctly.
When the URL trailing path matches one of the commit's files, that file
is moved to the top of the FileDiffList rather than scrolled to. Avoids
estimateSize-based scroll inaccuracy and works regardless of which side
of a rename the URL points to.
Lightweight diff viewer short-circuits with "Diff too large to display"
for files containing lines over 1000 chars, matching the cutoff in
lightweightCodeHighlighter.
PathHeader's breadcrumb measurement reserved 175px for "copy button and
padding"; the actual reservation needed is ~40px. Reduced so breadcrumbs
no longer collapse prematurely on wide layouts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Lift `truncateSha` (was a private helper in getDiffToolComponent) to `lib/utils` so PathHeader can reuse it. The branch/ref display now renders a 40-char SHA as `abc1234`, preserving any `^` / `~N` suffix. - Hide the `·` separator and the path's CopyIconButton when there's no path (repo root). Previously a dangling `·` and copy button rendered with nothing between them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
brendan-kellam
force-pushed
the
bkellam/commit-diffs
branch
from
bb56039 to
5c36e73
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Chores