From c50312c868b6d24406b5358445c7807bb82b47fa Mon Sep 17 00:00:00 2001 From: JRoy <10731363+JRoy@users.noreply.github.com> Date: Thu, 3 Jul 2025 23:32:45 -0700 Subject: [PATCH 1/4] Add support for making PR comments --- bun.lock | 3 + web/package.json | 1 + .../lib/components/diff/CommentDisplay.svelte | 429 +++++++++++++++++ .../lib/components/diff/CommentForm.svelte | 299 ++++++++++++ .../lib/components/diff/CommentThread.svelte | 251 ++++++++++ .../components/diff/ConciseDiffView.svelte | 441 +++++++++++++++++- .../settings-popover/SettingsPopover.svelte | 6 +- web/src/lib/diff-viewer-multi-file.svelte.ts | 181 ++++++- web/src/lib/github.svelte.ts | 221 ++++++++- web/src/routes/+page.svelte | 58 ++- 10 files changed, 1853 insertions(+), 37 deletions(-) create mode 100644 web/src/lib/components/diff/CommentDisplay.svelte create mode 100644 web/src/lib/components/diff/CommentForm.svelte create mode 100644 web/src/lib/components/diff/CommentThread.svelte diff --git a/bun.lock b/bun.lock index d3c647a..34559cd 100644 --- a/bun.lock +++ b/bun.lock @@ -10,6 +10,7 @@ "dependencies": { "bits-ui": "^2.4.1", "chroma-js": "^3.1.2", + "date-fns": "^4.1.0", "diff": "^8.0.2", "luxon": "^3.6.1", "runed": "^0.28.0", @@ -500,6 +501,8 @@ "data-uri-to-buffer": ["data-uri-to-buffer@2.0.2", "", {}, "sha512-ND9qDTLc6diwj+Xe5cdAgVTbLVdXbtxTJRXRhli8Mowuaan+0EJOtdqJ0QCHNSSPyoXGx9HX2/VMnKeC34AChA=="], + "date-fns": ["date-fns@4.1.0", "", {}, "sha512-Ukq0owbQXxa/U3EGtsdVBkR1w7KOQ5gIBqdH2hkvknzZPYvBxb/aa6E8L7tmjFtkwZBu3UXBbjIgPo/Ez4xaNg=="], + "debug": ["debug@4.4.1", "", { "dependencies": { "ms": "^2.1.3" } }, "sha512-KcKCqiftBJcZr++7ykoDIEwSa3XWowTfNPo92BYxjXiyYEVrUQh2aLyhxBCwww+heortUFxEJYcRzosstTEBYQ=="], "deep-eql": ["deep-eql@5.0.2", "", {}, "sha512-h5k/5U50IJJFpzfL6nO9jaaumfjO/f2NjK/oYB2Djzm4p9L+3T9qWpZqZ2hAbLPuuYq9wrU08WQyBTL5GbPk5Q=="], diff --git a/web/package.json b/web/package.json index 10b772e..f99042a 100644 --- a/web/package.json +++ b/web/package.json @@ -48,6 +48,7 @@ "dependencies": { "bits-ui": "^2.4.1", "chroma-js": "^3.1.2", + "date-fns": "^4.1.0", "diff": "^8.0.2", "luxon": "^3.6.1", "runed": "^0.28.0", diff --git a/web/src/lib/components/diff/CommentDisplay.svelte b/web/src/lib/components/diff/CommentDisplay.svelte new file mode 100644 index 0000000..1c0b37c --- /dev/null +++ b/web/src/lib/components/diff/CommentDisplay.svelte @@ -0,0 +1,429 @@ + + +
+
+ {comment.user.login} +
+ + {comment.user.login} + + + {formatTimestamp(comment.created_at)} + + {#if comment.created_at !== comment.updated_at} + (edited) + {/if} + {#if comment.html_url} +
+ • + View on GitHub +
+ {/if} +
+ + {#if (canEdit || canDelete) && !isEditing} +
+ {#if canEdit} + + {/if} + {#if canDelete} + + {/if} +
+ {/if} +
+ + {#if error} +
+ {error} +
+ {/if} + + {#if isEditing} +
+ +
+
+ + Use Cmd/Ctrl + Enter to save, Escape to cancel +
+
+ + +
+
+
+ {:else} +
+

{comment.body}

+
+ {/if} +
+ + diff --git a/web/src/lib/components/diff/CommentForm.svelte b/web/src/lib/components/diff/CommentForm.svelte new file mode 100644 index 0000000..0e1f491 --- /dev/null +++ b/web/src/lib/components/diff/CommentForm.svelte @@ -0,0 +1,299 @@ + + +
+ {#if error} +
+ {error} +
+ {/if} + + {#if isMultilineComment} +
+ + Commenting on lines {orderedLines.startLine}-{orderedLines.endLine} ({orderedLines.startSide === orderedLines.endSide + ? orderedLines.endSide + : `${orderedLines.startSide} to ${orderedLines.endSide}`}) +
+ {/if} + + + +
+
+ + Use Cmd/Ctrl + Enter to submit +
+ +
+ {#if onCancel} + + {/if} + + +
+
+
+ + diff --git a/web/src/lib/components/diff/CommentThread.svelte b/web/src/lib/components/diff/CommentThread.svelte new file mode 100644 index 0000000..2d48861 --- /dev/null +++ b/web/src/lib/components/diff/CommentThread.svelte @@ -0,0 +1,251 @@ + + +
+
+ + +
+ {thread.position.path}:{lineRangeDisplay} ({sideDisplay}) +
+
+ + {#if !isCollapsed} +
+ + {#each topLevelComments as comment (comment.id)} + + {/each} + + + {#if replies.length > 0} +
+
+ Replies +
+ {#each replies as reply (reply.id)} + + {/each} +
+ {/if} + + + {#if hasToken && topLevelComments.length > 0} +
+ {#if showReplyForm} + (showReplyForm = false)} + onSubmit={handleReplyAdded} + /> + {:else} + + {/if} +
+ {/if} +
+ {/if} +
+ + diff --git a/web/src/lib/components/diff/ConciseDiffView.svelte b/web/src/lib/components/diff/ConciseDiffView.svelte index a0fe7eb..b3f835d 100644 --- a/web/src/lib/components/diff/ConciseDiffView.svelte +++ b/web/src/lib/components/diff/ConciseDiffView.svelte @@ -13,6 +13,10 @@ type SearchSegment, } from "$lib/components/diff/concise-diff-view.svelte"; import Spinner from "$lib/components/Spinner.svelte"; + import CommentThread from "./CommentThread.svelte"; + import CommentForm from "./CommentForm.svelte"; + import type { CommentThread as CommentThreadType } from "$lib/diff-viewer-multi-file.svelte"; + import { getGithubToken, type GithubPRComment } from "$lib/github.svelte"; import { type StructuredPatch } from "diff"; import { onDestroy } from "svelte"; import { type MutableValue } from "$lib/util"; @@ -31,7 +35,26 @@ activeSearchResult = -1, cache, cacheKey, - }: ConciseDiffViewProps = $props(); + showComments = false, + commentsForLine, + onCommentAdded, + onCommentUpdated, + onCommentDeleted, + filePath, + owner, + repo, + prNumber, + }: ConciseDiffViewProps & { + showComments?: boolean; + commentsForLine?: (line: number, side: "LEFT" | "RIGHT") => CommentThreadType[]; + onCommentAdded?: (comment: GithubPRComment) => void; + onCommentUpdated?: (comment: GithubPRComment) => void; + onCommentDeleted?: (commentId: number) => void; + filePath?: string; + owner?: string; + repo?: string; + prNumber?: string; + } = $props(); const parsedPatch: Promise = $derived.by(async () => { if (rawPatchContent !== undefined) { @@ -53,6 +76,20 @@ cacheKey: box.with(() => cacheKey), }); + let hoveredLineKey = $state(null); + let showNewCommentForm = $state(null); // "line:side" format + + // Multiline comment selection state + let isSelecting = $state(false); + let selectionStart = $state<{ line: PatchLine; side: "LEFT" | "RIGHT" } | null>(null); + let selectionEnd = $state<{ line: PatchLine; side: "LEFT" | "RIGHT" } | null>(null); + let isDragging = $state(false); + let dragThreshold = 5; // pixels + let dragStartPosition = $state<{ x: number; y: number } | null>(null); + + const hasToken = $derived(!!getGithubToken()); + const canAddComments = $derived(showComments && hasToken && owner && repo && prNumber && filePath); + function getDisplayLineNo(line: PatchLine, num: number | undefined) { if (line.type == PatchLineType.HEADER) { return "..."; @@ -61,6 +98,149 @@ } } + function getLineKey(line: PatchLine, side: "LEFT" | "RIGHT"): string { + const lineNum = side === "LEFT" ? line.oldLineNo : line.newLineNo; + return `${lineNum}:${side}`; + } + + function getLineNumber(line: PatchLine, side: "LEFT" | "RIGHT"): number | undefined { + return side === "LEFT" ? line.oldLineNo : line.newLineNo; + } + + function handleAddComment(line: PatchLine, side: "LEFT" | "RIGHT") { + const lineKey = getLineKey(line, side); + + // If there's already a comment form open for a different line, close it and start new + if (showNewCommentForm && showNewCommentForm !== lineKey) { + showNewCommentForm = lineKey; + clearSelection(); + return; + } + + // Check if this is a multiline selection + if (selectionStart && selectionEnd && (selectionStart.line !== selectionEnd.line || selectionStart.side !== selectionEnd.side)) { + // Create multiline comment + const startLineNum = getLineNumber(selectionStart.line, selectionStart.side); + const endLineNum = getLineNumber(selectionEnd.line, selectionEnd.side); + + if (startLineNum !== undefined && endLineNum !== undefined) { + // Ensure proper order (start should be before end) + let actualStart = selectionStart; + let actualEnd = selectionEnd; + + if (startLineNum > endLineNum || (startLineNum === endLineNum && selectionStart.side === "RIGHT" && selectionEnd.side === "LEFT")) { + actualStart = selectionEnd; + actualEnd = selectionStart; + } + + // Update the selection state with the correct order + selectionStart = actualStart; + selectionEnd = actualEnd; + + showNewCommentForm = getLineKey(actualEnd.line, actualEnd.side); + clearSelection(); + return; + } + } + + showNewCommentForm = lineKey; + clearSelection(); + } + + function handleCommentFormCancel() { + showNewCommentForm = null; + clearSelection(); + } + + function handleCommentSubmitted(comment: GithubPRComment) { + showNewCommentForm = null; + clearSelection(); + onCommentAdded?.(comment); + } + + function handleCommentUpdated(comment: GithubPRComment) { + onCommentUpdated?.(comment); + } + + function handleCommentDeleted(commentId: number) { + onCommentDeleted?.(commentId); + } + + // Mouse event handlers for drag selection + function handleMouseDown(event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT") { + if (!canAddComments || line.type === PatchLineType.HEADER) return; + + const lineNum = getLineNumber(line, side); + if (lineNum === undefined) return; + + // Store initial position for drag threshold + dragStartPosition = { x: event.clientX, y: event.clientY }; + + isSelecting = true; + isDragging = false; // Don't set to true until we actually start dragging + selectionStart = { line, side }; + selectionEnd = { line, side }; + } + + function handleMouseMove(event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT") { + if (!isSelecting || !canAddComments || line.type === PatchLineType.HEADER) return; + + const lineNum = getLineNumber(line, side); + if (lineNum === undefined) return; + + // Check if we've moved far enough to start dragging + if (!isDragging && dragStartPosition) { + const dx = event.clientX - dragStartPosition.x; + const dy = event.clientY - dragStartPosition.y; + const distance = Math.sqrt(dx * dx + dy * dy); + + if (distance > dragThreshold) { + isDragging = true; + event.preventDefault(); + } + } + + if (isDragging) { + selectionEnd = { line, side }; + } + } + + function handleMouseUp() { + if (!isSelecting) return; + + // If we were dragging and have a multiline selection, open comment form + if (isDragging && selectionStart && selectionEnd && (selectionStart.line !== selectionEnd.line || selectionStart.side !== selectionEnd.side)) { + // Open comment form for multiline selection + const startLineNum = getLineNumber(selectionStart.line, selectionStart.side); + const endLineNum = getLineNumber(selectionEnd.line, selectionEnd.side); + + if (startLineNum !== undefined && endLineNum !== undefined) { + // Ensure proper order (start should be before end) + let actualStart = selectionStart; + let actualEnd = selectionEnd; + + // If dragging from bottom to top, swap the selection + if (startLineNum > endLineNum || (startLineNum === endLineNum && selectionStart.side === "RIGHT" && selectionEnd.side === "LEFT")) { + actualStart = selectionEnd; + actualEnd = selectionStart; + } + + // Update the selection state with the correct order + selectionStart = actualStart; + selectionEnd = actualEnd; + + showNewCommentForm = getLineKey(actualEnd.line, actualEnd.side); + isSelecting = false; + isDragging = false; + dragStartPosition = null; + return; + } + } + + // Otherwise clear selection + clearSelection(); + } + let searchResultElements: HTMLSpanElement[] = $state([]); let didInitialJump = $state(false); let scheduledJump: ReturnType | undefined = undefined; @@ -135,8 +315,76 @@ } return segments; }); + + function isLineInSelection(line: PatchLine, side: "LEFT" | "RIGHT"): boolean { + if (!selectionStart || !selectionEnd) return false; + + const lineNum = getLineNumber(line, side); + if (lineNum === undefined) return false; + + const startLineNum = getLineNumber(selectionStart.line, selectionStart.side); + const endLineNum = getLineNumber(selectionEnd.line, selectionEnd.side); + + if (startLineNum === undefined || endLineNum === undefined) return false; + + // Determine actual start and end (handle both directions) + let actualStartLineNum = startLineNum; + let actualEndLineNum = endLineNum; + let actualStartSide = selectionStart.side; + let actualEndSide = selectionEnd.side; + + // If dragging from bottom to top, swap the order + if (startLineNum > endLineNum || (startLineNum === endLineNum && selectionStart.side === "RIGHT" && selectionEnd.side === "LEFT")) { + actualStartLineNum = endLineNum; + actualEndLineNum = startLineNum; + actualStartSide = selectionEnd.side; + actualEndSide = selectionStart.side; + } + + // For same side selection + if (actualStartSide === actualEndSide && side === actualStartSide) { + return lineNum >= actualStartLineNum && lineNum <= actualEndLineNum; + } + + // For cross-side selection (left to right or right to left) + if (actualStartSide !== actualEndSide) { + if (side === actualStartSide) { + return lineNum >= actualStartLineNum; + } else if (side === actualEndSide) { + return lineNum <= actualEndLineNum; + } + } + + return false; + } + + function clearSelection() { + isSelecting = false; + selectionStart = null; + selectionEnd = null; + isDragging = false; + dragStartPosition = null; + } + + function handleClick(event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT") { + // Don't handle click if we just finished dragging + if (isDragging) { + event.preventDefault(); + return; + } + + // Don't handle click if there's a multiline selection (from drag) + if (selectionStart && selectionEnd && (selectionStart.line !== selectionEnd.line || selectionStart.side !== selectionEnd.side)) { + event.preventDefault(); + return; + } + + handleAddComment(line, side); + } + + {#snippet lineContent(line: PatchLine, lineType: PatchLineTypeProps, innerLineType: InnerPatchLineTypeProps)} {#if line.lineBreak}
{:else} @@ -187,19 +435,118 @@ {/await} {/snippet} +{#snippet commentButton(line: PatchLine, side: "LEFT" | "RIGHT")} + {@const lineKey = getLineKey(line, side)} + {@const lineNum = getLineNumber(line, side)} + {@const isSelected = isLineInSelection(line, side)} + {#if canAddComments && lineNum !== undefined && line.type !== PatchLineType.HEADER} + + {/if} +{/snippet} + {#snippet renderLine(line: PatchLine, hunkIndex: number, lineIndex: number)} {@const lineType = patchLineTypeProps[line.type]} -
-
{getDisplayLineNo(line, line.oldLineNo)}
+ {@const leftLineKey = getLineKey(line, "LEFT")} + {@const rightLineKey = getLineKey(line, "RIGHT")} + {@const leftSelected = isLineInSelection(line, "LEFT")} + {@const rightSelected = isLineInSelection(line, "RIGHT")} +
(hoveredLineKey = leftLineKey)} + onmouseleave={() => (hoveredLineKey = null)} + > +
{getDisplayLineNo(line, line.oldLineNo)}
+ {@render commentButton(line, "LEFT")}
-
-
{getDisplayLineNo(line, line.newLineNo)}
+
(hoveredLineKey = rightLineKey)} + onmouseleave={() => (hoveredLineKey = null)} + > +
{getDisplayLineNo(line, line.newLineNo)}
+ {@render commentButton(line, "RIGHT")}
-
+
{@render lineContentWrapper(line, hunkIndex, lineIndex, lineType, innerPatchLineTypeProps[line.innerPatchLineType])}
{/snippet} +{#snippet renderComments(line: PatchLine)} + {#if showComments && commentsForLine && filePath} + {@const leftLineNum = line.oldLineNo} + {@const rightLineNum = line.newLineNo} + {@const leftComments = leftLineNum ? commentsForLine(leftLineNum, "LEFT") : []} + {@const rightComments = rightLineNum ? commentsForLine(rightLineNum, "RIGHT") : []} + {@const allComments = [...leftComments, ...rightComments]} + + {#if allComments.length > 0} +
+
+
+
+ {#each allComments as thread (thread.id)} + + {/each} +
+
+ {/if} + + {#if showNewCommentForm} + {@const leftKey = getLineKey(line, "LEFT")} + {@const rightKey = getLineKey(line, "RIGHT")} + {#if showNewCommentForm === leftKey || showNewCommentForm === rightKey} + {@const isLeft = showNewCommentForm === leftKey} + {@const lineNum = isLeft ? line.oldLineNo : line.newLineNo} + {@const currentSide = isLeft ? "LEFT" : "RIGHT"} + {#if lineNum !== undefined} + {@const startLineNum = selectionStart ? getLineNumber(selectionStart.line, selectionStart.side) : undefined} + {@const startSide = selectionStart?.side} + {@const hasMultilineSelection = + selectionStart && selectionEnd && (selectionStart.line !== selectionEnd.line || selectionStart.side !== selectionEnd.side)} +
+
+
+
+ +
+
+ {/if} + {/if} + {/if} + {/if} +{/snippet} + {#await Promise.all([view.rootStyle, view.diffViewerPatch])}
{:then [rootStyle, diffViewerPatch]} @@ -211,6 +558,7 @@ {#each diffViewerPatch.hunks as hunk, hunkIndex (hunkIndex)} {#each hunk.lines as line, lineIndex (lineIndex)} {@render renderLine(line, hunkIndex, lineIndex)} + {@render renderComments(line)} {/each} {/each}
@@ -256,6 +604,9 @@ text-align: end; vertical-align: top; text-wrap: nowrap; + position: relative; + z-index: 0; + padding: 0 24px 0 8px; } .prefix { @@ -267,4 +618,82 @@ left: -0.75rem; top: 0; } + + .line-cell { + position: relative; + line-height: 1.25rem; + min-height: 1.25rem; + max-height: 1.25rem; + overflow: visible; + z-index: 1; + } + + .line-cell.line-selected { + background: var(--color-blue-100) !important; + border: 1px solid var(--color-blue-300); + border-radius: 2px; + } + + .line-selected { + background: var(--color-blue-50) !important; + } + + .comment-button { + position: absolute; + top: 50%; + right: 2px; + transform: translateY(-50%); + background: var(--color-primary); + border: none; + border-radius: 50%; + width: 18px; + height: 18px; + display: flex; + align-items: center; + justify-content: center; + cursor: pointer; + opacity: 0; + transition: opacity 0.2s ease; + color: white; + font-size: 10px; + z-index: 10; + box-shadow: 0 1px 3px rgba(0, 0, 0, 0.2); + } + + .comment-button.visible { + opacity: 1; + } + + .comment-button.selected { + opacity: 1; + background: var(--color-blue-600); + box-shadow: 0 2px 6px rgba(0, 0, 0, 0.3); + } + + .comment-button:hover { + background: var(--color-primary-hover); + } + + .comment-button.selected:hover { + background: var(--color-blue-700); + } + + .comment-row { + display: grid; + grid-column: 1 / -1; + grid-template-columns: subgrid; + background: var(--color-neutral-1); + border-top: 1px solid var(--color-border); + border-bottom: 1px solid var(--color-border); + } + + .comment-spacer { + background: var(--color-neutral-2); + border-right: 1px solid var(--color-border); + } + + .comments-container { + padding: 2px 6px; + background: var(--color-neutral-1); + } diff --git a/web/src/lib/components/settings-popover/SettingsPopover.svelte b/web/src/lib/components/settings-popover/SettingsPopover.svelte index 4b733d2..3e1d9ea 100644 --- a/web/src/lib/components/settings-popover/SettingsPopover.svelte +++ b/web/src/lib/components/settings-popover/SettingsPopover.svelte @@ -1,7 +1,3 @@ - - +
@@ -125,16 +43,16 @@ {/if}
- {#if (canEdit || canDelete) && !isEditing} + {#if (displayState.canEdit || displayState.canDelete) && !displayState.isEditing}
- {#if canEdit} - {/if} - {#if canDelete} -
- {#if error} + {#if displayState.error}
- {error} + {displayState.error}
{/if} - {#if isEditing} + {#if displayState.isEditing}
- +
Use Cmd/Ctrl + Enter to save, Escape to cancel
- - + + {/if} -
diff --git a/web/src/lib/components/diff/CommentThread.svelte b/web/src/lib/components/diff/CommentThread.svelte index 2d48861..d51e7c1 100644 --- a/web/src/lib/components/diff/CommentThread.svelte +++ b/web/src/lib/components/diff/CommentThread.svelte @@ -1,8 +1,9 @@
-
- {thread.position.path}:{lineRangeDisplay} ({sideDisplay}) + {thread.position.path}:{threadState.lineRangeDisplay} ({threadState.sideDisplay})
- {#if !isCollapsed} + {#if !threadState.collapsed}
- {#each topLevelComments as comment (comment.id)} - + {#each threadState.topLevelComments as comment (comment.id)} + {/each} - {#if replies.length > 0} + {#if threadState.replies.length > 0}
Replies
- {#each replies as reply (reply.id)} + {#each threadState.replies as reply (reply.id)} {/each}
{/if} - {#if hasToken && topLevelComments.length > 0} + {#if threadState.hasToken && threadState.topLevelComments.length > 0}
- {#if showReplyForm} + {#if threadState.showReplyForm} (showReplyForm = false)} - onSubmit={handleReplyAdded} + onCancel={() => (threadState.showReplyForm = false)} + onSubmit={threadState.handleReplyAdded} /> {:else} - + {/if}
{/if} diff --git a/web/src/lib/components/diff/ConciseDiffView.svelte b/web/src/lib/components/diff/ConciseDiffView.svelte index ad71751..d4c692a 100644 --- a/web/src/lib/components/diff/ConciseDiffView.svelte +++ b/web/src/lib/components/diff/ConciseDiffView.svelte @@ -80,9 +80,10 @@ let showNewCommentForm = $state(null); // "line:side" format // Multiline comment selection state + // Note: GitHub API requires multiline comments to be within the same hunk, so we track hunkIndex let isSelecting = $state(false); - let selectionStart = $state<{ line: PatchLine; side: "LEFT" | "RIGHT" } | null>(null); - let selectionEnd = $state<{ line: PatchLine; side: "LEFT" | "RIGHT" } | null>(null); + let selectionStart = $state<{ line: PatchLine; side: "LEFT" | "RIGHT"; hunkIndex: number } | null>(null); + let selectionEnd = $state<{ line: PatchLine; side: "LEFT" | "RIGHT"; hunkIndex: number } | null>(null); let isDragging = $state(false); let dragThreshold = 5; // pixels let dragStartPosition = $state<{ x: number; y: number } | null>(null); @@ -167,7 +168,7 @@ } // Mouse event handlers for drag selection - function handleMouseDown(event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT") { + function handleMouseDown(event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT", hunkIndex: number) { if (!canAddComments || line.type === PatchLineType.HEADER) return; const lineNum = getLineNumber(line, side); @@ -178,16 +179,21 @@ isSelecting = true; isDragging = false; // Don't set to true until we actually start dragging - selectionStart = { line, side }; - selectionEnd = { line, side }; + selectionStart = { line, side, hunkIndex }; + selectionEnd = { line, side, hunkIndex }; } - function handleMouseMove(event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT") { + function handleMouseMove(event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT", hunkIndex: number) { if (!isSelecting || !canAddComments || line.type === PatchLineType.HEADER) return; const lineNum = getLineNumber(line, side); if (lineNum === undefined) return; + // Don't allow selection across different hunks + if (selectionStart && selectionStart.hunkIndex !== hunkIndex) { + return; + } + // Check if we've moved far enough to start dragging if (!isDragging && dragStartPosition) { const dx = event.clientX - dragStartPosition.x; @@ -201,7 +207,7 @@ } if (isDragging) { - selectionEnd = { line, side }; + selectionEnd = { line, side, hunkIndex }; } } @@ -316,9 +322,12 @@ return segments; }); - function isLineInSelection(line: PatchLine, side: "LEFT" | "RIGHT"): boolean { + function isLineInSelection(line: PatchLine, side: "LEFT" | "RIGHT", hunkIndex: number): boolean { if (!selectionStart || !selectionEnd) return false; + // Only lines in the same hunk as the selection can be selected + if (selectionStart.hunkIndex !== hunkIndex) return false; + const lineNum = getLineNumber(line, side); if (lineNum === undefined) return false; @@ -435,18 +444,18 @@ {/await} {/snippet} -{#snippet commentButton(line: PatchLine, side: "LEFT" | "RIGHT")} +{#snippet commentButton(line: PatchLine, side: "LEFT" | "RIGHT", hunkIndex: number)} {@const lineKey = getLineKey(line, side)} {@const lineNum = getLineNumber(line, side)} - {@const isSelected = isLineInSelection(line, side)} + {@const isSelected = isLineInSelection(line, side, hunkIndex)} {#if canAddComments && lineNum !== undefined && line.type !== PatchLineType.HEADER} + + + +
+ +
+ +
@@ -96,6 +197,9 @@ border-radius: 3px; padding: 4px 6px; margin: 2px 0; + display: flex; + flex-direction: column; + gap: 0; } .error-message { @@ -121,24 +225,103 @@ gap: 4px; } + @variant dark { + .multiline-indicator { + background: var(--color-blue-900); + border-color: var(--color-blue-700); + color: var(--color-blue-300); + } + } + + .textarea-container { + display: flex; + flex-direction: column; + border: 1px solid var(--color-border); + border-radius: 2px; + overflow: hidden; + background: var(--color-neutral); + } + + .textarea-container:focus-within { + border-color: var(--color-primary); + box-shadow: 0 0 0 1px var(--color-primary-20); + } + + .markdown-toolbar { + display: flex; + align-items: center; + gap: 2px; + padding: 4px 6px; + background: var(--color-neutral-2); + border-bottom: 1px solid var(--color-border); + } + + .toolbar-button { + display: flex; + align-items: center; + gap: 2px; + padding: 2px 4px; + border: none; + background: none; + border-radius: 2px; + cursor: pointer; + color: var(--color-em-med); + font-size: 0.7rem; + transition: all 0.2s; + } + + .toolbar-button:hover { + background: var(--color-neutral-3); + color: var(--color-em-high); + } + + .toolbar-button:active { + background: var(--color-neutral-4); + } + + .toolbar-button:disabled { + opacity: 0.5; + cursor: not-allowed; + } + + .toolbar-button.suggestion-button { + color: var(--color-blue-600); + } + + .toolbar-button.suggestion-button:hover { + background: var(--color-blue-100); + color: var(--color-blue-700); + } + + .toolbar-label { + font-size: 0.6875rem; + font-weight: 500; + } + + .toolbar-divider { + width: 1px; + height: 16px; + background: var(--color-border); + margin: 0 4px; + } + .comment-textarea { width: 100%; - min-height: 48px; + height: 48px; padding: 6px 8px; - border: 1px solid var(--color-border); - border-radius: 2px; + border: none; + border-radius: 0; font-family: inherit; font-size: 0.75rem; line-height: 1.3; - resize: vertical; + resize: none; + overflow-y: auto; background: var(--color-neutral); color: var(--color-em-high); } .comment-textarea:focus { outline: none; - border-color: var(--color-primary); - box-shadow: 0 0 0 1px var(--color-primary-20); } .comment-textarea:disabled { diff --git a/web/src/lib/components/diff/CommentThread.svelte b/web/src/lib/components/diff/CommentThread.svelte index d51e7c1..779f842 100644 --- a/web/src/lib/components/diff/CommentThread.svelte +++ b/web/src/lib/components/diff/CommentThread.svelte @@ -13,11 +13,22 @@ onCommentAdded?: (comment: GithubPRComment) => void; onCommentUpdated?: (comment: GithubPRComment) => void; onCommentDeleted?: (commentId: number) => void; + originalContentForSuggestion?: string; } - let { thread, owner, repo, prNumber, onCommentAdded, onCommentUpdated, onCommentDeleted }: Props = $props(); + let { thread, owner, repo, prNumber, onCommentAdded, onCommentUpdated, onCommentDeleted, originalContentForSuggestion }: Props = $props(); const threadState = new CommentThreadState(thread, onCommentAdded, onCommentUpdated, onCommentDeleted); + + // Calculate the starting line for suggestions based on the thread's position + // For multiline comments, use the start_line; for single-line comments, use the thread's position line + const startLine = $derived.by(() => { + const firstComment = threadState.topLevelComments[0]; + if (firstComment && firstComment.start_line !== undefined && firstComment.start_line !== null) { + return firstComment.start_line; + } + return thread.position.line; + });
@@ -33,12 +44,19 @@ {thread.position.path}:{threadState.lineRangeDisplay} ({threadState.sideDisplay})
- {#if !threadState.collapsed}
{#each threadState.topLevelComments as comment (comment.id)} - + {/each} @@ -55,6 +73,8 @@ {repo} onCommentUpdated={threadState.handleCommentUpdated} onCommentDeleted={threadState.handleCommentDeleted} + {originalContentForSuggestion} + {startLine} /> {/each}
diff --git a/web/src/lib/components/diff/ConciseDiffView.svelte b/web/src/lib/components/diff/ConciseDiffView.svelte index d4c692a..ba6ba98 100644 --- a/web/src/lib/components/diff/ConciseDiffView.svelte +++ b/web/src/lib/components/diff/ConciseDiffView.svelte @@ -11,6 +11,8 @@ type PatchLineTypeProps, patchLineTypeProps, type SearchSegment, + type DiffViewerPatch, + type DiffViewerPatchHunk, } from "$lib/components/diff/concise-diff-view.svelte"; import Spinner from "$lib/components/Spinner.svelte"; import CommentThread from "./CommentThread.svelte"; @@ -18,7 +20,7 @@ import type { CommentThread as CommentThreadType } from "$lib/diff-viewer-multi-file.svelte"; import { getGithubToken, type GithubPRComment } from "$lib/github.svelte"; import { type StructuredPatch } from "diff"; - import { onDestroy } from "svelte"; + import { onDestroy, setContext } from "svelte"; import { type MutableValue } from "$lib/util"; import { box } from "svelte-toolbelt"; @@ -56,6 +58,13 @@ prNumber?: string; } = $props(); + setContext("diff-settings", { + syntaxHighlighting, + syntaxHighlightingTheme, + wordDiffs, + lineWrap, + }); + const parsedPatch: Promise = $derived.by(async () => { if (rawPatchContent !== undefined) { return parseSinglePatch(rawPatchContent); @@ -108,6 +117,34 @@ return side === "LEFT" ? line.oldLineNo : line.newLineNo; } + function getOriginalContentForThread(thread: CommentThreadType, hunk: DiffViewerPatchHunk): string { + const firstComment = thread.comments[0]; + if (!firstComment) return ""; + + const isMultiline = firstComment.start_line !== undefined && firstComment.start_line !== null && firstComment.start_line !== firstComment.line; + + const startLine = isMultiline ? firstComment.start_line! : thread.position.line; + const endLine = thread.position.line; + + const selectedLinesContent: string[] = []; + + for (const line of hunk.lines) { + if (line.type === PatchLineType.ADD || line.type === PatchLineType.CONTEXT) { + const lineNum = line.newLineNo; + if (lineNum !== undefined && lineNum >= startLine && lineNum <= endLine) { + const content = extractLineContent(line); + selectedLinesContent.push(content); + } + } + } + + return selectedLinesContent.join("\n"); + } + + function extractLineContent(line: PatchLine): string { + return line.content.map((segment) => segment.text || "").join(""); + } + function handleAddComment(line: PatchLine, side: "LEFT" | "RIGHT") { const lineKey = getLineKey(line, side); @@ -336,6 +373,11 @@ if (startLineNum === undefined || endLineNum === undefined) return false; + // For single line selection (same line and side) + if (selectionStart.line === selectionEnd.line && selectionStart.side === selectionEnd.side) { + return line === selectionStart.line && side === selectionStart.side; + } + // Determine actual start and end (handle both directions) let actualStartLineNum = startLineNum; let actualEndLineNum = endLineNum; @@ -355,12 +397,12 @@ return lineNum >= actualStartLineNum && lineNum <= actualEndLineNum; } - // For cross-side selection (left to right or right to left) + // For cross-side selection, only include the exact start and end lines if (actualStartSide !== actualEndSide) { - if (side === actualStartSide) { - return lineNum >= actualStartLineNum; - } else if (side === actualEndSide) { - return lineNum <= actualEndLineNum; + if (side === actualStartSide && line === selectionStart.line) { + return true; + } else if (side === actualEndSide && line === selectionEnd.line) { + return true; } } @@ -390,6 +432,39 @@ handleAddComment(line, side); } + + function getSelectionInfo(diffViewerPatch: DiffViewerPatch): { content: string; containsDeletedLines: boolean } { + if (!selectionStart || !selectionEnd) return { content: "", containsDeletedLines: false }; + + let containsDeletedLines = false; + const selectedLines: string[] = []; + const hunk = diffViewerPatch.hunks[selectionStart.hunkIndex]; + + if (hunk) { + for (let lineIndex = 0; lineIndex < hunk.lines.length; lineIndex++) { + const line = hunk.lines[lineIndex]; + + // Skip header and spacer lines + if (line.type === PatchLineType.HEADER || line.type === PatchLineType.SPACER) continue; + + // Check if this line is in the selection using the same logic as isLineInSelection + if (isLineInSelection(line, "LEFT", selectionStart.hunkIndex) || isLineInSelection(line, "RIGHT", selectionStart.hunkIndex)) { + if (line.type === PatchLineType.REMOVE) { + containsDeletedLines = true; + } + const content = extractLineContent(line); + // Remove diff prefix if present + let cleanContent = content; + if (content.startsWith("+") || content.startsWith("-") || content.startsWith(" ")) { + cleanContent = content.substring(1); + } + selectedLines.push(cleanContent); + } + } + } + + return { content: selectedLines.join("\n"), containsDeletedLines }; + } @@ -444,23 +519,59 @@ {/await} {/snippet} -{#snippet commentButton(line: PatchLine, side: "LEFT" | "RIGHT", hunkIndex: number)} - {@const lineKey = getLineKey(line, side)} - {@const lineNum = getLineNumber(line, side)} - {@const isSelected = isLineInSelection(line, side, hunkIndex)} - {#if canAddComments && lineNum !== undefined && line.type !== PatchLineType.HEADER} - +{#snippet commentButton(line: PatchLine, hunkIndex: number, isRightColumn: boolean)} + {@const leftLineKey = getLineKey(line, "LEFT")} + {@const rightLineKey = getLineKey(line, "RIGHT")} + {@const leftLineNum = getLineNumber(line, "LEFT")} + {@const rightLineNum = getLineNumber(line, "RIGHT")} + {@const leftSelected = isLineInSelection(line, "LEFT", hunkIndex)} + {@const rightSelected = isLineInSelection(line, "RIGHT", hunkIndex)} + {@const isThisLineHovered = hoveredLineKey === leftLineKey || hoveredLineKey === rightLineKey} + + {#if canAddComments && (leftLineNum !== undefined || rightLineNum !== undefined) && line.type !== PatchLineType.HEADER} + {#if line.type === PatchLineType.REMOVE && isRightColumn} + + + {:else if line.type === PatchLineType.ADD && isRightColumn} + + + {:else if line.type === PatchLineType.CONTEXT && isRightColumn} + + + {/if} {/if} {/snippet} @@ -473,33 +584,43 @@
(hoveredLineKey = leftLineKey)} + onmouseenter={() => { + if (line.oldLineNo !== undefined) { + hoveredLineKey = leftLineKey; + } + }} onmouseleave={() => (hoveredLineKey = null)} aria-label="Add comment" role="button" tabindex="0" >
{getDisplayLineNo(line, line.oldLineNo)}
- {@render commentButton(line, "LEFT", hunkIndex)}
(hoveredLineKey = rightLineKey)} + onmouseenter={() => { + if (line.newLineNo !== undefined) { + hoveredLineKey = rightLineKey; + } else if (line.oldLineNo !== undefined) { + // For deleted lines (no right line number), use left line key + hoveredLineKey = leftLineKey; + } + }} onmouseleave={() => (hoveredLineKey = null)} aria-label="Add comment" role="button" tabindex="0" >
{getDisplayLineNo(line, line.newLineNo)}
- {@render commentButton(line, "RIGHT", hunkIndex)} + {@render commentButton(line, hunkIndex, true)}
{@render lineContentWrapper(line, hunkIndex, lineIndex, lineType, innerPatchLineTypeProps[line.innerPatchLineType])}
{/snippet} -{#snippet renderComments(line: PatchLine)} +{#snippet renderComments(line: PatchLine, diffViewerPatch: DiffViewerPatch, hunk: DiffViewerPatchHunk)} {#if showComments && commentsForLine && filePath} {@const leftLineNum = line.oldLineNo} {@const rightLineNum = line.newLineNo} @@ -513,6 +634,7 @@
{#each allComments as thread (thread.id)} + {@const originalContentForSuggestion = getOriginalContentForThread(thread, hunk)} {/each}
@@ -539,6 +662,20 @@ {@const startSide = selectionStart?.side} {@const hasMultilineSelection = selectionStart && selectionEnd && (selectionStart.line !== selectionEnd.line || selectionStart.side !== selectionEnd.side)} + {@const selectionInfo = getSelectionInfo(diffViewerPatch)} + {@const singleLineContent = + !hasMultilineSelection && selectionInfo.content === "" + ? (() => { + const content = extractLineContent(line); + // Remove diff prefix if present + if (content.startsWith("+") || content.startsWith("-") || content.startsWith(" ")) { + return content.substring(1); + } + return content; + })() + : selectionInfo.content} + {@const singleLineIsDeleted = !hasMultilineSelection && line.type === PatchLineType.REMOVE} + {@const shouldDisableSuggestions = selectionInfo.containsDeletedLines || singleLineIsDeleted}
@@ -552,6 +689,8 @@ side={currentSide} startLine={hasMultilineSelection ? startLineNum : undefined} startSide={hasMultilineSelection ? startSide : undefined} + selectedContent={singleLineContent} + disableSuggestions={shouldDisableSuggestions} onSubmit={handleCommentSubmitted} onCancel={handleCommentFormCancel} /> @@ -574,7 +713,7 @@ {#each diffViewerPatch.hunks as hunk, hunkIndex (hunkIndex)} {#each hunk.lines as line, lineIndex (lineIndex)} {@render renderLine(line, hunkIndex, lineIndex)} - {@render renderComments(line)} + {@render renderComments(line, diffViewerPatch, hunk)} {/each} {/each}
@@ -622,7 +761,7 @@ text-wrap: nowrap; position: relative; z-index: 0; - padding: 0 24px 0 8px; + padding: 0 8px 0 8px; } .prefix { @@ -645,13 +784,13 @@ } .line-cell.line-selected { - background: var(--color-blue-100) !important; - border: 1px solid var(--color-blue-300); + background: var(--select-bg, var(--color-blue-100)) !important; + border: 1px solid var(--color-primary); border-radius: 2px; } .line-selected { - background: var(--color-blue-50) !important; + background: var(--select-bg, var(--color-blue-50)) !important; } .comment-button { @@ -687,11 +826,11 @@ } .comment-button:hover { - background: var(--color-primary-hover); + background: var(--color-primary); } .comment-button.selected:hover { - background: var(--color-blue-700); + background: var(--color-blue-600); } .comment-row { diff --git a/web/src/lib/components/diff/MarkdownRenderer.svelte b/web/src/lib/components/diff/MarkdownRenderer.svelte new file mode 100644 index 0000000..fcd1854 --- /dev/null +++ b/web/src/lib/components/diff/MarkdownRenderer.svelte @@ -0,0 +1,254 @@ + + +
+ {#each parts as part, index (index)} + {#if part.type === "markdown"} + + {@html renderMarkdown(part.text)} + {:else if part.type === "suggestion"} + + {/if} + {/each} +
+ + diff --git a/web/src/lib/components/diff/SuggestionDiff.svelte b/web/src/lib/components/diff/SuggestionDiff.svelte new file mode 100644 index 0000000..39411ff --- /dev/null +++ b/web/src/lib/components/diff/SuggestionDiff.svelte @@ -0,0 +1,103 @@ + + +
+ +
+ + diff --git a/web/src/lib/components/diff/comment-state.svelte.ts b/web/src/lib/components/diff/comment-state.svelte.ts index 6d615d7..a00fa20 100644 --- a/web/src/lib/components/diff/comment-state.svelte.ts +++ b/web/src/lib/components/diff/comment-state.svelte.ts @@ -1,6 +1,13 @@ -import { type GithubPRComment, getGithubToken, getGithubUsername, createGithubPRComment, replyToGithubPRComment, updateGithubPRComment, deleteGithubPRComment } from "$lib/github.svelte"; +import { + type GithubPRComment, + getGithubToken, + getGithubUsername, + createGithubPRComment, + replyToGithubPRComment, + updateGithubPRComment, + deleteGithubPRComment, +} from "$lib/github.svelte"; import type { CommentThread } from "$lib/diff-viewer-multi-file.svelte"; -import type { PatchLine } from "$lib/components/diff/concise-diff-view.svelte"; export interface CommentFormProps { owner: string; @@ -27,10 +34,7 @@ export class CommentFormState { readonly canSubmit = $derived(this.text.trim().length > 0 && !this.isSubmitting); readonly isReply = $derived(!!this.props.replyToId); readonly isMultilineComment = $derived( - !this.isReply && - this.props.startLine !== undefined && - this.props.startSide !== undefined && - this.props.startLine !== this.props.line + !this.isReply && this.props.startLine !== undefined && this.props.startSide !== undefined && this.props.startLine !== this.props.line, ); constructor(props: CommentFormProps, onSubmit?: (comment: GithubPRComment) => void, onCancel?: () => void) { @@ -42,44 +46,44 @@ export class CommentFormState { // Ensure correct line ordering for GitHub API readonly orderedLines = $derived.by(() => { if (!this.isMultilineComment || this.props.startLine === undefined || this.props.startSide === undefined || this.props.line === undefined) { - return { - startLine: this.props.startLine, - startSide: this.props.startSide, - endLine: this.props.line, - endSide: this.props.side + return { + startLine: this.props.startLine, + startSide: this.props.startSide, + endLine: this.props.line, + endSide: this.props.side, }; } // GitHub API requires start_line < line if (this.props.startLine < this.props.line) { - return { - startLine: this.props.startLine, - startSide: this.props.startSide, - endLine: this.props.line, - endSide: this.props.side + return { + startLine: this.props.startLine, + startSide: this.props.startSide, + endLine: this.props.line, + endSide: this.props.side, }; } else if (this.props.startLine > this.props.line) { - return { - startLine: this.props.line, - startSide: this.props.side, - endLine: this.props.startLine, - endSide: this.props.startSide + return { + startLine: this.props.line, + startSide: this.props.side, + endLine: this.props.startLine, + endSide: this.props.startSide, }; } else { // Same line number - order by side (LEFT before RIGHT) if (this.props.startSide === "LEFT" && this.props.side === "RIGHT") { - return { - startLine: this.props.startLine, - startSide: this.props.startSide, - endLine: this.props.line, - endSide: this.props.side + return { + startLine: this.props.startLine, + startSide: this.props.startSide, + endLine: this.props.line, + endSide: this.props.side, }; } else { - return { - startLine: this.props.line, - startSide: this.props.side, - endLine: this.props.startLine, - endSide: this.props.startSide + return { + startLine: this.props.line, + startSide: this.props.side, + endLine: this.props.startLine, + endSide: this.props.startSide, }; } } @@ -101,14 +105,7 @@ export class CommentFormState { let comment: GithubPRComment; if (this.isReply) { - comment = await replyToGithubPRComment( - token, - this.props.owner, - this.props.repo, - this.props.prNumber, - this.props.replyToId!, - this.text.trim() - ); + comment = await replyToGithubPRComment(token, this.props.owner, this.props.repo, this.props.prNumber, this.props.replyToId!, this.text.trim()); } else { if (!this.props.path || this.props.line === undefined || !this.props.side) { throw new Error("Path, line, and side are required for new comments"); @@ -189,7 +186,7 @@ export class CommentDisplayState { owner: string, repo: string, onUpdated?: (comment: GithubPRComment) => void, - onDeleted?: (commentId: number) => void + onDeleted?: (commentId: number) => void, ) { this.comment = comment; this.owner = owner; @@ -226,13 +223,7 @@ export class CommentDisplayState { this.error = null; try { - const updatedComment = await updateGithubPRComment( - token, - this.owner, - this.repo, - this.comment.id, - this.editText.trim() - ); + const updatedComment = await updateGithubPRComment(token, this.owner, this.repo, this.comment.id, this.editText.trim()); this.onUpdated?.(updatedComment); this.isEditing = false; this.editText = ""; @@ -288,14 +279,14 @@ export class CommentThreadState { private readonly onCommentDeleted?: (commentId: number) => void; readonly hasToken = $derived(!!getGithubToken()); - readonly topLevelComments = $derived(this.thread.comments.filter(comment => !comment.in_reply_to_id)); - readonly replies = $derived(this.thread.comments.filter(comment => comment.in_reply_to_id)); + readonly topLevelComments = $derived(this.thread.comments.filter((comment) => !comment.in_reply_to_id)); + readonly replies = $derived(this.thread.comments.filter((comment) => comment.in_reply_to_id)); constructor( thread: CommentThread, onCommentAdded?: (comment: GithubPRComment) => void, onCommentUpdated?: (comment: GithubPRComment) => void, - onCommentDeleted?: (commentId: number) => void + onCommentDeleted?: (commentId: number) => void, ) { this.thread = thread; this.collapsed = thread.collapsed; @@ -306,10 +297,7 @@ export class CommentThreadState { readonly isMultilineThread = $derived.by(() => { const firstComment = this.topLevelComments[0]; - return firstComment && - firstComment.start_line !== undefined && - firstComment.start_line !== null && - firstComment.start_line !== firstComment.line; + return firstComment && firstComment.start_line !== undefined && firstComment.start_line !== null && firstComment.start_line !== firstComment.line; }); readonly lineRangeDisplay = $derived.by(() => { @@ -359,199 +347,3 @@ export class CommentThreadState { this.onCommentDeleted?.(commentId); }; } - -export interface LineSelection { - line: PatchLine; - side: "LEFT" | "RIGHT"; -} - -export class CommentSelectionState { - isSelecting: boolean = $state(false); - selectionStart: LineSelection | null = $state(null); - selectionEnd: LineSelection | null = $state(null); - isDragging: boolean = $state(false); - dragStartPosition: { x: number; y: number } | null = $state(null); - showNewCommentForm: string | null = $state(null); // "line:side" format - hoveredLineKey: string | null = $state(null); - - private readonly dragThreshold = 5; // pixels - private readonly canAddComments: boolean; - private readonly getLineNumber: (line: PatchLine, side: "LEFT" | "RIGHT") => number | undefined; - private readonly getLineKey: (line: PatchLine, side: "LEFT" | "RIGHT") => string; - - constructor( - canAddComments: boolean, - getLineNumber: (line: PatchLine, side: "LEFT" | "RIGHT") => number | undefined, - getLineKey: (line: PatchLine, side: "LEFT" | "RIGHT") => string - ) { - this.canAddComments = canAddComments; - this.getLineNumber = getLineNumber; - this.getLineKey = getLineKey; - } - - handleMouseDown = (event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT"): void => { - if (!this.canAddComments) return; - - const lineNum = this.getLineNumber(line, side); - if (lineNum === undefined) return; - - this.dragStartPosition = { x: event.clientX, y: event.clientY }; - this.isSelecting = true; - this.isDragging = false; - this.selectionStart = { line, side }; - this.selectionEnd = { line, side }; - }; - - handleMouseMove = (event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT"): void => { - if (!this.isSelecting || !this.canAddComments) return; - - const lineNum = this.getLineNumber(line, side); - if (lineNum === undefined) return; - - if (!this.isDragging && this.dragStartPosition) { - const dx = event.clientX - this.dragStartPosition.x; - const dy = event.clientY - this.dragStartPosition.y; - const distance = Math.sqrt(dx * dx + dy * dy); - - if (distance > this.dragThreshold) { - this.isDragging = true; - event.preventDefault(); - } - } - - if (this.isDragging) { - this.selectionEnd = { line, side }; - } - }; - - handleMouseUp = (): void => { - if (!this.isSelecting) return; - - if (this.isDragging && this.selectionStart && this.selectionEnd) { - const hasMultilineSelection = this.selectionStart.line !== this.selectionEnd.line || - this.selectionStart.side !== this.selectionEnd.side; - - if (hasMultilineSelection) { - const startLineNum = this.getLineNumber(this.selectionStart.line, this.selectionStart.side); - const endLineNum = this.getLineNumber(this.selectionEnd.line, this.selectionEnd.side); - - if (startLineNum !== undefined && endLineNum !== undefined) { - // Ensure proper order - if (startLineNum > endLineNum || - (startLineNum === endLineNum && this.selectionStart.side === "RIGHT" && this.selectionEnd.side === "LEFT")) { - const temp = this.selectionStart; - this.selectionStart = this.selectionEnd; - this.selectionEnd = temp; - } - - this.showNewCommentForm = this.getLineKey(this.selectionEnd.line, this.selectionEnd.side); - this.isSelecting = false; - this.isDragging = false; - this.dragStartPosition = null; - return; - } - } - } - - this.clearSelection(); - }; - - handleClick = (event: MouseEvent, line: PatchLine, side: "LEFT" | "RIGHT"): void => { - if (this.isDragging) { - event.preventDefault(); - return; - } - - if (this.selectionStart && this.selectionEnd && - (this.selectionStart.line !== this.selectionEnd.line || this.selectionStart.side !== this.selectionEnd.side)) { - event.preventDefault(); - return; - } - - this.handleAddComment(line, side); - }; - - handleAddComment = (line: PatchLine, side: "LEFT" | "RIGHT"): void => { - const lineKey = this.getLineKey(line, side); - - if (this.showNewCommentForm && this.showNewCommentForm !== lineKey) { - this.showNewCommentForm = lineKey; - this.clearSelection(); - return; - } - - if (this.selectionStart && this.selectionEnd && - (this.selectionStart.line !== this.selectionEnd.line || this.selectionStart.side !== this.selectionEnd.side)) { - const startLineNum = this.getLineNumber(this.selectionStart.line, this.selectionStart.side); - const endLineNum = this.getLineNumber(this.selectionEnd.line, this.selectionEnd.side); - - if (startLineNum !== undefined && endLineNum !== undefined) { - if (startLineNum > endLineNum || - (startLineNum === endLineNum && this.selectionStart.side === "RIGHT" && this.selectionEnd.side === "LEFT")) { - const temp = this.selectionStart; - this.selectionStart = this.selectionEnd; - this.selectionEnd = temp; - } - - this.showNewCommentForm = this.getLineKey(this.selectionEnd.line, this.selectionEnd.side); - this.clearSelection(); - return; - } - } - - this.showNewCommentForm = lineKey; - this.clearSelection(); - }; - - isLineInSelection(line: PatchLine, side: "LEFT" | "RIGHT"): boolean { - if (!this.selectionStart || !this.selectionEnd) return false; - - const lineNum = this.getLineNumber(line, side); - if (lineNum === undefined) return false; - - const startLineNum = this.getLineNumber(this.selectionStart.line, this.selectionStart.side); - const endLineNum = this.getLineNumber(this.selectionEnd.line, this.selectionEnd.side); - - if (startLineNum === undefined || endLineNum === undefined) return false; - - let actualStartLineNum = startLineNum; - let actualEndLineNum = endLineNum; - let actualStartSide = this.selectionStart.side; - let actualEndSide = this.selectionEnd.side; - - if (startLineNum > endLineNum || - (startLineNum === endLineNum && this.selectionStart.side === "RIGHT" && this.selectionEnd.side === "LEFT")) { - actualStartLineNum = endLineNum; - actualEndLineNum = startLineNum; - actualStartSide = this.selectionEnd.side; - actualEndSide = this.selectionStart.side; - } - - if (actualStartSide === actualEndSide && side === actualStartSide) { - return lineNum >= actualStartLineNum && lineNum <= actualEndLineNum; - } - - if (actualStartSide !== actualEndSide) { - if (side === actualStartSide) { - return lineNum >= actualStartLineNum; - } else if (side === actualEndSide) { - return lineNum <= actualEndLineNum; - } - } - - return false; - } - - clearSelection = (): void => { - this.isSelecting = false; - this.selectionStart = null; - this.selectionEnd = null; - this.isDragging = false; - this.dragStartPosition = null; - }; - - cancelCommentForm = (): void => { - this.showNewCommentForm = null; - this.clearSelection(); - }; -} \ No newline at end of file diff --git a/web/src/lib/components/diff/concise-diff-view.svelte.ts b/web/src/lib/components/diff/concise-diff-view.svelte.ts index a89c861..7ce548b 100644 --- a/web/src/lib/components/diff/concise-diff-view.svelte.ts +++ b/web/src/lib/components/diff/concise-diff-view.svelte.ts @@ -13,8 +13,8 @@ import { import { guessLanguageFromExtension, type MutableValue, type ReadableBoxedValues } from "$lib/util"; import type { IRawThemeSetting } from "shiki/textmate"; import chroma from "chroma-js"; -import { getEffectiveGlobalTheme } from "$lib/theme.svelte"; import { onDestroy } from "svelte"; +import { getEffectiveGlobalTheme } from "$lib/theme.svelte"; export const DEFAULT_THEME_LIGHT: BundledTheme = "github-light-default"; export const DEFAULT_THEME_DARK: BundledTheme = "github-dark-default"; @@ -198,6 +198,11 @@ class LineProcessor { private async processInternal() { for (let i = 0; i < this.contentLines.length; i++) { + // Yield to the event loop every couple hundred lines so large diffs don't freeze the UI + if (i % 200 === 0) { + // Using a micro-task is enough to let rendering catch up without noticeable slowdown + await Promise.resolve(); + } const lineText = this.contentLines[i]; const oldState = this.state; @@ -835,132 +840,145 @@ function makeTransparent(hex: string | undefined) { return `rgba(${rgb[0]}, ${rgb[1]}, ${rgb[2]}, 0.5)`; } -export async function getBaseColors(themeKey: BundledTheme | undefined, syntaxHighlighting: boolean): Promise { - const theme = await getTheme(themeKey); - if (!syntaxHighlighting || !theme) { - let styles = ""; - if (getEffectiveGlobalTheme() === "dark") { - // Make sure tailwind emits these props - // "text-green-600 text-red-600 text-green-700 text-red-700 text-green-800 text-red-800 text-blue-800" - styles += ` - --hunk-header-bg-themed: var(--color-gray-800); - --select-bg-themed: var(--color-blue-800); - - --inserted-text-bg-themed: var(--color-green-700); - --removed-text-bg-themed: var(--color-red-700); - --inserted-line-bg-themed: var(--color-green-800); - --removed-line-bg-themed: var(--color-red-800); - --inner-inserted-line-bg-themed: var(--color-green-600); - --inner-removed-line-bg-themed: var(--color-red-600); - --inner-inserted-line-fg-themed: var(--color-green-300); - --inner-removed-line-fg-themed: var(--color-red-300); - `; - } else { - styles += ` - --inserted-text-bg-themed: var(--color-green-400); - --removed-text-bg-themed: var(--color-red-400); - --inserted-line-bg-themed: var(--color-green-100); - --removed-line-bg-themed: var(--color-red-100); - --inner-inserted-line-bg-themed: var(--color-green-300); - --inner-removed-line-bg-themed: var(--color-red-300); - --inner-inserted-line-fg-themed: var(--color-green-800); - --inner-removed-line-fg-themed: var(--color-red-800); - `; - } - return styles; +// Memoization cache to avoid recomputing base colors for every diff view +const baseColorCache: Map> = new Map(); + +export async function getBaseColors(themeKey: BundledTheme | undefined, syntaxHighlighting: boolean, isDark: boolean): Promise { + const cacheKey = `${String(themeKey)}|${syntaxHighlighting}|${isDark}`; + if (baseColorCache.has(cacheKey)) { + return baseColorCache.get(cacheKey)!; } - const tokenColors = theme.default.tokenColors || []; - const style: Map = new Map(); + const compute = (async () => { + const theme = await getTheme(themeKey); + if (!syntaxHighlighting || !theme) { + let styles = ""; + if (isDark) { + // Make sure tailwind emits these props + // "text-green-600 text-red-600 text-green-700 text-red-700 text-green-800 text-red-800 text-blue-800" + styles += ` + --hunk-header-bg-themed: var(--color-gray-800); + --select-bg-themed: var(--color-blue-800); + + --inserted-text-bg-themed: var(--color-green-700); + --removed-text-bg-themed: var(--color-red-700); + --inserted-line-bg-themed: var(--color-green-800); + --removed-line-bg-themed: var(--color-red-800); + --inner-inserted-line-bg-themed: var(--color-green-600); + --inner-removed-line-bg-themed: var(--color-red-600); + --inner-inserted-line-fg-themed: var(--color-green-300); + --inner-removed-line-fg-themed: var(--color-red-300); + `; + } else { + styles += ` + --inserted-text-bg-themed: var(--color-green-400); + --removed-text-bg-themed: var(--color-red-400); + --inserted-line-bg-themed: var(--color-green-100); + --removed-line-bg-themed: var(--color-red-100); + --inner-inserted-line-bg-themed: var(--color-green-300); + --inner-removed-line-bg-themed: var(--color-red-300); + --inner-inserted-line-fg-themed: var(--color-green-800); + --inner-removed-line-fg-themed: var(--color-red-800); + `; + } + return styles; + } + const tokenColors = theme.default.tokenColors || []; + + const style: Map = new Map(); - // Find the foreground/default text color for the theme - const foundFg = extractColor(theme.default, { color: "editor.foreground" }); - if (foundFg) { - style.set("--editor-fg-themed", foundFg); - makeLCHVars("--editor-foreground", foundFg, style); - } else { - let globalScope = tokenColors.find((t) => t.scope === undefined); - if (!globalScope) { - // Tokenize something to force Shiki to run it's fix for 'broken' themes - await codeToTokens("hi", { theme: theme.default, lang: "text" }); - globalScope = tokenColors.find((t) => t.scope === undefined); + // Find the foreground/default text color for the theme + const foundFg = extractColor(theme.default, { color: "editor.foreground" }); + if (foundFg) { + style.set("--editor-fg-themed", foundFg); + makeLCHVars("--editor-foreground", foundFg, style); + } else { + let globalScope = tokenColors.find((t) => t.scope === undefined); + if (!globalScope) { + // Tokenize something to force Shiki to run it's fix for 'broken' themes + await codeToTokens("hi", { theme: theme.default, lang: "text" }); + globalScope = tokenColors.find((t) => t.scope === undefined); + } + const globalFg = globalScope?.settings.foreground; + if (globalFg) { + style.set("--editor-fg-themed", globalFg); + makeLCHVars("--editor-foreground", globalFg, style); + } else { + console.error("No foreground color found in theme"); + } + } + + // These colors are mostly universal + style.set("--editor-bg-themed", extractColor(theme.default, { color: "editor.background" })); + makeLCHVars("--editor-background", style.get("--editor-bg-themed"), style); + style.set("--select-bg-themed", extractColor(theme.default, { color: "editor.selectionBackground" })); + + // Don't use these - just add chroma to the inner diff highlight below for consistency + // These are also applied to the line by VSCode...? + // style.set("--inserted-text-bg-themed", extractColor(theme.default, { color: "diffEditor.insertedTextBackground" })); + // style.set("--removed-text-bg-themed", extractColor(theme.default, { color: "diffEditor.removedTextBackground" })); + + // 1) Try diffEditor.insertedLineBackground for inserted line highlight color + // 2) Try editorGutter.addedBackground for inserted line highlight color + // 3) Try markup.inserted scope bg for inserted line highlight color + // 4) Try markup.inserted scope fg for inserted line text color + let insertLineBg = extractColor(theme.default, { color: "diffEditor.insertedLineBackground" }); + if (!insertLineBg) { + insertLineBg = extractColor(theme.default, { color: "editorGutter.addedBackground", modifier: makeTransparent }); + } + if (!insertLineBg) { + insertLineBg = extractColor(theme.default, { bgTokenScope: "markup.inserted", modifier: makeTransparent }); } - const globalFg = globalScope?.settings.foreground; - if (globalFg) { - style.set("--editor-fg-themed", globalFg); - makeLCHVars("--editor-foreground", globalFg, style); + if (insertLineBg) { + style.set("--inserted-line-bg-themed", insertLineBg); + style.set("--inner-inserted-line-bg-themed", moreChroma(insertLineBg, 0.5)); + style.set("--inserted-text-bg-themed", darken(moreChroma(insertLineBg, 1.25), 0.25)); + + // Only use the fg color if we have a bg color -- otherwise it will conflict with the top level diff add/remove lines + // Increase chroma to match our adjustments to bg color above + style.set("--inner-inserted-line-fg-themed", moreChroma(extractColor(theme.default, { fgTokenScope: "markup.inserted" }))); } else { - console.error("No foreground color found in theme"); + style.set("--inserted-line-fg-themed", extractColor(theme.default, { fgTokenScope: "markup.inserted" })); } - } - // These colors are mostly universal - style.set("--editor-bg-themed", extractColor(theme.default, { color: "editor.background" })); - makeLCHVars("--editor-background", style.get("--editor-bg-themed"), style); - style.set("--select-bg-themed", extractColor(theme.default, { color: "editor.selectionBackground" })); - - // Don't use these - just add chroma to the inner diff highlight below for consistency - // These are also applied to the line by VSCode...? - // style.set("--inserted-text-bg-themed", extractColor(theme.default, { color: "diffEditor.insertedTextBackground" })); - // style.set("--removed-text-bg-themed", extractColor(theme.default, { color: "diffEditor.removedTextBackground" })); - - // 1) Try diffEditor.insertedLineBackground for inserted line highlight color - // 2) Try editorGutter.addedBackground for inserted line highlight color - // 3) Try markup.inserted scope bg for inserted line highlight color - // 4) Try markup.inserted scope fg for inserted line text color - let insertLineBg = extractColor(theme.default, { color: "diffEditor.insertedLineBackground" }); - if (!insertLineBg) { - insertLineBg = extractColor(theme.default, { color: "editorGutter.addedBackground", modifier: makeTransparent }); - } - if (!insertLineBg) { - insertLineBg = extractColor(theme.default, { bgTokenScope: "markup.inserted", modifier: makeTransparent }); - } - if (insertLineBg) { - style.set("--inserted-line-bg-themed", insertLineBg); - style.set("--inner-inserted-line-bg-themed", moreChroma(insertLineBg, 0.5)); - style.set("--inserted-text-bg-themed", darken(moreChroma(insertLineBg, 1.25), 0.25)); - - // Only use the fg color if we have a bg color -- otherwise it will conflict with the top level diff add/remove lines - // Increase chroma to match our adjustments to bg color above - style.set("--inner-inserted-line-fg-themed", moreChroma(extractColor(theme.default, { fgTokenScope: "markup.inserted" }))); - } else { - style.set("--inserted-line-fg-themed", extractColor(theme.default, { fgTokenScope: "markup.inserted" })); - } + // 1) Try diffEditor.removedLineBackground for removed line highlight color + // 2) Try editorGutter.deletedBackground for removed line highlight color + // 3) Try markup.deleted scope bg for removed line highlight color + // 4) Try markup.deleted scope fg for removed line text color + let removeLineBg = extractColor(theme.default, { color: "diffEditor.removedLineBackground" }); + if (!removeLineBg) { + removeLineBg = extractColor(theme.default, { color: "editorGutter.deletedBackground", modifier: makeTransparent }); + } + if (!removeLineBg) { + removeLineBg = extractColor(theme.default, { bgTokenScope: "markup.deleted", modifier: makeTransparent }); + } + if (removeLineBg) { + style.set("--removed-line-bg-themed", removeLineBg); + style.set("--inner-removed-line-bg-themed", moreChroma(removeLineBg, 0.5)); + style.set("--removed-text-bg-themed", darken(moreChroma(removeLineBg, 1.25), 0.25)); + + // Only use the fg color if we have a bg color -- otherwise it will conflict with the top level diff add/remove lines + // Increase chroma to match our adjustments to bg color above + style.set("--inner-removed-line-fg-themed", moreChroma(extractColor(theme.default, { fgTokenScope: "markup.deleted" }))); + } else { + style.set("--removed-line-fg-themed", extractColor(theme.default, { fgTokenScope: "markup.deleted" })); + } - // 1) Try diffEditor.removedLineBackground for removed line highlight color - // 2) Try editorGutter.deletedBackground for removed line highlight color - // 3) Try markup.deleted scope bg for removed line highlight color - // 4) Try markup.deleted scope fg for removed line text color - let removeLineBg = extractColor(theme.default, { color: "diffEditor.removedLineBackground" }); - if (!removeLineBg) { - removeLineBg = extractColor(theme.default, { color: "editorGutter.deletedBackground", modifier: makeTransparent }); - } - if (!removeLineBg) { - removeLineBg = extractColor(theme.default, { bgTokenScope: "markup.deleted", modifier: makeTransparent }); - } - if (removeLineBg) { - style.set("--removed-line-bg-themed", removeLineBg); - style.set("--inner-removed-line-bg-themed", moreChroma(removeLineBg, 0.5)); - style.set("--removed-text-bg-themed", darken(moreChroma(removeLineBg, 1.25), 0.25)); - - // Only use the fg color if we have a bg color -- otherwise it will conflict with the top level diff add/remove lines - // Increase chroma to match our adjustments to bg color above - style.set("--inner-removed-line-fg-themed", moreChroma(extractColor(theme.default, { fgTokenScope: "markup.deleted" }))); - } else { - style.set("--removed-line-fg-themed", extractColor(theme.default, { fgTokenScope: "markup.deleted" })); - } + // One or both of these is often missing, see ConciseDiffView.svelte