Conversation
The patch file was for version 1.2.4 but the dependency has been upgraded to 1.2.7. The patch may no longer be needed or may need to be updated for the new version. Removing it to avoid potential conflicts.
|
/review |
![orc-review[bot]](https://avatars.githubusercontent.com/u/32248437?s=60&v=4){kind=small}
| const commentsByPath = useMemo(() => { | ||
| const next = new Map<string, ReturnType<typeof toLineAnnotations>>(); | ||
| for (const target of targets) { | ||
| next.set( |
There was a problem hiding this comment.
This uses only target.path to collect comments, but a file can appear in both the staged and unstaged targets at the same time. Since CommentItem carries a bucket, comments added on the staged diff will also be rendered on the unstaged diff for the same path (and vice versa), especially if the line numbers overlap. Filter the comments by target.bucket before converting them to annotations.
|
/review |
There was a problem hiding this comment.
LGTM with minor suggestions. Major refactor upgrading @pierre/diffs library from 1.1.15 to 1.2.7 and moving from single-file to multi-file diff architecture. Changes include new components (ChangesCodeViewDiffPane, PullRequestCodeViewDiffPane, RepoActionErrorDialog), refactored diagnostics handling (useMultiDiffDiagnostics), and Git error logging improvements. The PR is large (3413 insertions, 934 deletions across 40 files) but the architecture changes appear well-structured and follow established patterns.
| `cwd: ${input.repoPath}`, | ||
| `exit code: ${input.code ?? "unknown"}`, | ||
| "", | ||
| "stderr:", |
There was a problem hiding this comment.
writeGitCommandErrorLog catches all errors silently. If the temp directory cannot be created or written to, users won't get any indication that error logging failed. Consider at least logging to console.error for debugging purposes, though this is acceptable for a diagnostic feature.
| if (diagnostics.length === 0) { | ||
| closePopover(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The effect runs on every change to diagnosticsByLineByItem and repaints all rendered root nodes. This could be expensive if diagnostics change frequently while many diffs are rendered. Consider memoizing or comparing diagnostic arrays to avoid unnecessary repaints, though current implementation is functionally correct.
1 participant
No description provided.