mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): allow diff comments on cross-side selections (#23322)
This commit is contained in:
Danielle Maywood
@@ -0,0 +1,344 @@
|
||||
import type {
|
||||
DiffLineAnnotation,
|
||||
FileDiffMetadata,
|
||||
SelectedLineRange,
|
||||
} from "@pierre/diffs";
|
||||
import { Button } from "components/Button/Button";
|
||||
import { ArrowUpIcon } from "lucide-react";
|
||||
import {
|
||||
type FC,
|
||||
type RefObject,
|
||||
useCallback,
|
||||
useLayoutEffect,
|
||||
useRef,
|
||||
useState,
|
||||
} from "react";
|
||||
import type { ChatMessageInputRef } from "./AgentChatInput";
|
||||
import type { DiffStyle } from "./DiffViewer";
|
||||
import { DiffViewer } from "./DiffViewer";
|
||||
import {
|
||||
annotationLineForBox,
|
||||
annotationSideForBox,
|
||||
type CommentBoxState,
|
||||
commentBoxFromRange,
|
||||
contentRangeForBox,
|
||||
selectedLinesForBox,
|
||||
} from "./diffCommentSelection";
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Diff content extraction
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Walk the parsed hunks for a file and collect code lines that fall
|
||||
* within `startLine..endLine` on the given side. For "additions"
|
||||
* lines are matched against addition line numbers (using
|
||||
* `hunk.additionStart`); for "deletions" against deletion line
|
||||
* numbers (using `hunk.deletionStart`). Context lines that fall
|
||||
* in range are included as well.
|
||||
*/
|
||||
export function extractDiffContent(
|
||||
parsedFiles: readonly FileDiffMetadata[],
|
||||
fileName: string,
|
||||
startLine: number,
|
||||
endLine: number,
|
||||
side: "additions" | "deletions",
|
||||
): string {
|
||||
const file = parsedFiles.find((f) => f.name === fileName);
|
||||
if (!file) return "";
|
||||
|
||||
const lines = side === "additions" ? file.additionLines : file.deletionLines;
|
||||
const collected: string[] = [];
|
||||
for (const hunk of file.hunks) {
|
||||
let addLine = hunk.additionStart;
|
||||
let delLine = hunk.deletionStart;
|
||||
|
||||
for (const block of hunk.hunkContent) {
|
||||
if (block.type === "context") {
|
||||
for (let i = 0; i < block.lines; i++) {
|
||||
const ln = side === "additions" ? addLine : delLine;
|
||||
if (ln >= startLine && ln <= endLine) {
|
||||
const idx =
|
||||
side === "additions"
|
||||
? block.additionLineIndex + i
|
||||
: block.deletionLineIndex + i;
|
||||
if (lines[idx] != null) collected.push(lines[idx]);
|
||||
}
|
||||
addLine++;
|
||||
delLine++;
|
||||
}
|
||||
} else {
|
||||
// ChangeContent block.
|
||||
if (side === "deletions") {
|
||||
for (let i = 0; i < block.deletions; i++) {
|
||||
if (delLine >= startLine && delLine <= endLine) {
|
||||
const line = lines[block.deletionLineIndex + i];
|
||||
if (line != null) collected.push(line);
|
||||
}
|
||||
delLine++;
|
||||
}
|
||||
// Addition lines in a change block still advance
|
||||
// the addition counter.
|
||||
addLine += block.additions;
|
||||
} else {
|
||||
// side === "additions"
|
||||
// Deletion lines in a change block still advance
|
||||
// the deletion counter.
|
||||
delLine += block.deletions;
|
||||
for (let i = 0; i < block.additions; i++) {
|
||||
if (addLine >= startLine && addLine <= endLine) {
|
||||
const line = lines[block.additionLineIndex + i];
|
||||
if (line != null) collected.push(line);
|
||||
}
|
||||
addLine++;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return collected.join("\n");
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Inline prompt input
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Inline input rendered as a diff annotation under the selected
|
||||
* line(s). Supports multiline via Shift+Enter. Enter submits,
|
||||
* Escape dismisses.
|
||||
*/
|
||||
export const InlinePromptInput: FC<{
|
||||
onSubmit: (text: string) => void;
|
||||
onCancel: () => void;
|
||||
}> = ({ onSubmit, onCancel }) => {
|
||||
const [text, setText] = useState("");
|
||||
const textareaRef = useRef<HTMLTextAreaElement>(null);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
textareaRef.current?.focus();
|
||||
}, []);
|
||||
|
||||
return (
|
||||
<div className="px-2 py-1.5">
|
||||
<div className="rounded-lg border border-border-default/80 bg-surface-secondary/45 p-1 shadow-sm has-[textarea:focus]:ring-2 has-[textarea:focus]:ring-content-link/40">
|
||||
<textarea
|
||||
ref={textareaRef}
|
||||
className="w-full resize-none border-none bg-transparent px-3 py-2 font-sans text-sm leading-5 text-content-primary placeholder:text-content-secondary outline-none ring-0 focus:outline-none focus:ring-0"
|
||||
placeholder="Add a comment..."
|
||||
rows={2}
|
||||
value={text}
|
||||
onChange={(e) => setText(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" && !e.shiftKey) {
|
||||
e.preventDefault();
|
||||
if (text.trim()) {
|
||||
onSubmit(text.trim());
|
||||
} else {
|
||||
onCancel();
|
||||
}
|
||||
}
|
||||
if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
onCancel();
|
||||
}
|
||||
}}
|
||||
/>
|
||||
<div className="flex items-end justify-between gap-2 pl-2.5 pr-1.5 pb-1.5">
|
||||
<span className="text-xs text-content-secondary">Esc to cancel</span>
|
||||
<Button
|
||||
size="icon"
|
||||
variant="default"
|
||||
className="size-7 rounded-full transition-colors [&>svg]:!size-4 [&>svg]:p-0"
|
||||
disabled={!text.trim()}
|
||||
onMouseDown={(e: React.MouseEvent) => {
|
||||
// Prevent blur from firing before click.
|
||||
e.preventDefault();
|
||||
}}
|
||||
onClick={() => {
|
||||
if (text.trim()) {
|
||||
onSubmit(text.trim());
|
||||
}
|
||||
}}
|
||||
>
|
||||
<ArrowUpIcon />
|
||||
<span className="sr-only">Add to chat</span>
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// CommentableDiffViewer
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
interface CommentableDiffViewerProps {
|
||||
/** Parsed file diffs to render. */
|
||||
parsedFiles: readonly FileDiffMetadata[];
|
||||
/** Whether the panel is in expanded mode. */
|
||||
isExpanded?: boolean;
|
||||
/** Loading state. */
|
||||
isLoading?: boolean;
|
||||
/** Error state. */
|
||||
error?: unknown;
|
||||
/** Empty state message. */
|
||||
emptyMessage?: string;
|
||||
/** Which diff rendering style to use. */
|
||||
diffStyle: DiffStyle;
|
||||
/** Ref to the chat message input for inserting comments. */
|
||||
chatInputRef?: RefObject<ChatMessageInputRef | null>;
|
||||
/** Scroll to a specific file. */
|
||||
scrollToFile?: string | null;
|
||||
/** Called after scrollToFile has been processed. */
|
||||
onScrollToFileComplete?: () => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* Wraps `DiffViewer` with inline commenting support. Click a line
|
||||
* number or select a range to open a comment input that inserts a
|
||||
* file reference chip and text into the chat input.
|
||||
*/
|
||||
export const CommentableDiffViewer: FC<CommentableDiffViewerProps> = ({
|
||||
parsedFiles,
|
||||
chatInputRef,
|
||||
...diffViewerProps
|
||||
}) => {
|
||||
// ---------------------------------------------------------------
|
||||
// Comment / annotation state
|
||||
// ---------------------------------------------------------------
|
||||
const [activeCommentBox, setActiveCommentBox] =
|
||||
useState<CommentBoxState | null>(null);
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// Line interaction callbacks
|
||||
// ---------------------------------------------------------------
|
||||
const handleLineNumberClick = useCallback(
|
||||
(
|
||||
fileName: string,
|
||||
props: {
|
||||
lineNumber: number;
|
||||
annotationSide: "additions" | "deletions";
|
||||
},
|
||||
) => {
|
||||
setActiveCommentBox({
|
||||
fileName,
|
||||
start: props.lineNumber,
|
||||
startSide: props.annotationSide,
|
||||
end: props.lineNumber,
|
||||
endSide: props.annotationSide,
|
||||
});
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
const handleLineSelected = useCallback(
|
||||
(
|
||||
fileName: string,
|
||||
range: {
|
||||
start: number;
|
||||
end: number;
|
||||
side?: "additions" | "deletions";
|
||||
endSide?: "additions" | "deletions";
|
||||
} | null,
|
||||
) => {
|
||||
const result = commentBoxFromRange(fileName, range);
|
||||
if (result === "ignore") return;
|
||||
setActiveCommentBox(result);
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// Annotation helpers
|
||||
// ---------------------------------------------------------------
|
||||
const getLineAnnotations = useCallback(
|
||||
(fileName: string): DiffLineAnnotation<string>[] => {
|
||||
if (activeCommentBox && activeCommentBox.fileName === fileName) {
|
||||
return [
|
||||
{
|
||||
side: annotationSideForBox(activeCommentBox),
|
||||
lineNumber: annotationLineForBox(activeCommentBox),
|
||||
metadata: "active-input",
|
||||
},
|
||||
];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
[activeCommentBox],
|
||||
);
|
||||
|
||||
const getSelectedLines = useCallback(
|
||||
(fileName: string): SelectedLineRange | null => {
|
||||
if (activeCommentBox && activeCommentBox.fileName === fileName) {
|
||||
return selectedLinesForBox(activeCommentBox);
|
||||
}
|
||||
return null;
|
||||
},
|
||||
[activeCommentBox],
|
||||
);
|
||||
|
||||
const handleCancelComment = useCallback(() => {
|
||||
setActiveCommentBox(null);
|
||||
}, []);
|
||||
|
||||
const handleSubmitComment = useCallback(
|
||||
(text: string) => {
|
||||
if (!activeCommentBox) return;
|
||||
const { startLine, endLine, side } = contentRangeForBox(activeCommentBox);
|
||||
const content = extractDiffContent(
|
||||
parsedFiles,
|
||||
activeCommentBox.fileName,
|
||||
startLine,
|
||||
endLine,
|
||||
side,
|
||||
);
|
||||
// Single imperative call -- chip inserted atomically
|
||||
// in one Lexical update. No rAF hack needed.
|
||||
chatInputRef?.current?.addFileReference({
|
||||
fileName: activeCommentBox.fileName,
|
||||
startLine,
|
||||
endLine,
|
||||
content,
|
||||
});
|
||||
if (text.trim()) {
|
||||
chatInputRef?.current?.insertText(text);
|
||||
}
|
||||
setActiveCommentBox(null);
|
||||
},
|
||||
[activeCommentBox, chatInputRef, parsedFiles],
|
||||
);
|
||||
|
||||
const renderAnnotation = useCallback(
|
||||
(annotation: DiffLineAnnotation<string>) => {
|
||||
if (annotation.metadata === "active-input") {
|
||||
if (!activeCommentBox) return null;
|
||||
return (
|
||||
<InlinePromptInput
|
||||
onSubmit={handleSubmitComment}
|
||||
onCancel={handleCancelComment}
|
||||
/>
|
||||
);
|
||||
}
|
||||
return null;
|
||||
},
|
||||
[activeCommentBox, handleSubmitComment, handleCancelComment],
|
||||
);
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// Render
|
||||
// ---------------------------------------------------------------
|
||||
return (
|
||||
<DiffViewer
|
||||
{...diffViewerProps}
|
||||
parsedFiles={parsedFiles}
|
||||
onLineNumberClick={handleLineNumberClick}
|
||||
onLineSelected={handleLineSelected}
|
||||
getLineAnnotations={getLineAnnotations}
|
||||
getSelectedLines={getSelectedLines}
|
||||
renderAnnotation={renderAnnotation}
|
||||
/>
|
||||
);
|
||||
};
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { DiffLineAnnotation, SelectedLineRange } from "@pierre/diffs";
|
||||
import { parsePatchFiles } from "@pierre/diffs";
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { fn } from "storybook/test";
|
||||
import { expect, fn, waitFor } from "storybook/test";
|
||||
import type { DiffStyle } from "./DiffViewer";
|
||||
import { DiffViewer } from "./DiffViewer";
|
||||
import { InlinePromptInput } from "./RemoteDiffPanel";
|
||||
@@ -150,3 +150,255 @@ export const WithAnnotation: Story = {
|
||||
),
|
||||
},
|
||||
};
|
||||
|
||||
// Diff with a change block (both deletions and additions) for
|
||||
// testing cross-side selection in split view.
|
||||
// biome-ignore format: raw diff string must preserve exact whitespace
|
||||
const changeDiff = [
|
||||
"diff --git a/src/config.ts b/src/config.ts",
|
||||
"index abc1234..def5678 100644",
|
||||
"--- a/src/config.ts",
|
||||
"+++ b/src/config.ts",
|
||||
"@@ -1,5 +1,5 @@",
|
||||
" const config = {",
|
||||
"- port: 3000,",
|
||||
"- host: \"localhost\",",
|
||||
"+ port: 8080,",
|
||||
"+ host: \"0.0.0.0\",",
|
||||
" debug: false,",
|
||||
" };",
|
||||
].join("\n");
|
||||
const changeFiles = parsePatchFiles(changeDiff).flatMap((p) => p.files);
|
||||
const changeFileName = changeFiles[0]?.name ?? "";
|
||||
|
||||
// Regression test: in split view, selecting from one side to the
|
||||
// other can produce a range where start === end numerically but
|
||||
// the sides differ (e.g. deletions line 2 → additions line 2).
|
||||
// Previously this was incorrectly treated as a single-line click
|
||||
// and the annotation was never shown.
|
||||
export const CrossSideAnnotation: Story = {
|
||||
args: {
|
||||
parsedFiles: changeFiles,
|
||||
diffStyle: "split",
|
||||
getSelectedLines: (fileName: string): SelectedLineRange | null => {
|
||||
if (fileName === changeFileName) {
|
||||
return {
|
||||
start: 2,
|
||||
end: 2,
|
||||
side: "deletions",
|
||||
endSide: "additions",
|
||||
};
|
||||
}
|
||||
return null;
|
||||
},
|
||||
getLineAnnotations: (fileName: string): DiffLineAnnotation<string>[] => {
|
||||
if (fileName === changeFileName) {
|
||||
return [
|
||||
{
|
||||
lineNumber: 2,
|
||||
side: "additions",
|
||||
metadata: "active-input",
|
||||
},
|
||||
];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
renderAnnotation: () => (
|
||||
<InlinePromptInput onSubmit={fn()} onCancel={fn()} />
|
||||
),
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
// The annotation renders via a slot in the light DOM of the
|
||||
// web component, so we can find the textarea directly.
|
||||
await waitFor(() => {
|
||||
const textarea = canvasElement.querySelector("textarea");
|
||||
expect(textarea).not.toBeNull();
|
||||
});
|
||||
},
|
||||
};
|
||||
|
||||
// Same regression scenario in unified view to ensure the
|
||||
// annotation also renders when diffStyle is "unified".
|
||||
export const CrossSideAnnotationUnified: Story = {
|
||||
args: {
|
||||
...CrossSideAnnotation.args,
|
||||
diffStyle: "unified",
|
||||
},
|
||||
play: CrossSideAnnotation.play,
|
||||
};
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Edge-case stories
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
// Play function shared by all annotation edge-case stories.
|
||||
const expectAnnotationTextarea = async ({
|
||||
canvasElement,
|
||||
}: {
|
||||
canvasElement: HTMLElement;
|
||||
}) => {
|
||||
await waitFor(() => {
|
||||
const textarea = canvasElement.querySelector("textarea");
|
||||
expect(textarea).not.toBeNull();
|
||||
});
|
||||
};
|
||||
|
||||
// Diff where deletion and addition line numbers are wildly
|
||||
// different (hunk header: @@ -508,4 +218,4 @@). Deletion
|
||||
// lines are 509-510, addition lines are 219-220.
|
||||
// biome-ignore format: raw diff string must preserve exact whitespace
|
||||
const mismatchedLinesDiff = [
|
||||
"diff --git a/src/big.ts b/src/big.ts",
|
||||
"index abc1234..def5678 100644",
|
||||
"--- a/src/big.ts",
|
||||
"+++ b/src/big.ts",
|
||||
"@@ -508,6 +218,6 @@ function process() {",
|
||||
" return result;",
|
||||
"- const old1 = true;",
|
||||
"- const old2 = false;",
|
||||
"+ const new1 = true;",
|
||||
"+ const new2 = false;",
|
||||
" cleanup();",
|
||||
" }",
|
||||
].join("\n");
|
||||
const mismatchedFiles = parsePatchFiles(mismatchedLinesDiff).flatMap(
|
||||
(p) => p.files,
|
||||
);
|
||||
const mismatchedFileName = mismatchedFiles[0]?.name ?? "";
|
||||
|
||||
// Cross-side selection where deletion line 509 maps to addition
|
||||
// line 219. The old code would Math.min/max these into a
|
||||
// nonsensical 290-line range.
|
||||
export const CrossSideMismatchedLineNumbers: Story = {
|
||||
args: {
|
||||
parsedFiles: mismatchedFiles,
|
||||
diffStyle: "split",
|
||||
getSelectedLines: (fileName: string): SelectedLineRange | null => {
|
||||
if (fileName === mismatchedFileName) {
|
||||
return {
|
||||
start: 509,
|
||||
end: 219,
|
||||
side: "deletions",
|
||||
endSide: "additions",
|
||||
};
|
||||
}
|
||||
return null;
|
||||
},
|
||||
getLineAnnotations: (fileName: string): DiffLineAnnotation<string>[] => {
|
||||
if (fileName === mismatchedFileName) {
|
||||
return [
|
||||
{
|
||||
lineNumber: 219,
|
||||
side: "additions",
|
||||
metadata: "active-input",
|
||||
},
|
||||
];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
renderAnnotation: () => (
|
||||
<InlinePromptInput onSubmit={fn()} onCancel={fn()} />
|
||||
),
|
||||
},
|
||||
play: expectAnnotationTextarea,
|
||||
};
|
||||
|
||||
// Same mismatched-line-number scenario in unified view.
|
||||
export const CrossSideMismatchedLineNumbersUnified: Story = {
|
||||
args: {
|
||||
...CrossSideMismatchedLineNumbers.args,
|
||||
diffStyle: "unified",
|
||||
},
|
||||
play: expectAnnotationTextarea,
|
||||
};
|
||||
|
||||
// Backward same-side selection (start > end). The user clicks
|
||||
// line 9 then shift-clicks line 5 on the additions side.
|
||||
// biome-ignore format: raw diff string must preserve exact whitespace
|
||||
const backwardSelectionDiff = [
|
||||
"diff --git a/src/utils.ts b/src/utils.ts",
|
||||
"index abc1234..def5678 100644",
|
||||
"--- a/src/utils.ts",
|
||||
"+++ b/src/utils.ts",
|
||||
"@@ -3,4 +3,9 @@",
|
||||
" import { foo } from \"./foo\";",
|
||||
" import { bar } from \"./bar\";",
|
||||
"+import { baz } from \"./baz\";",
|
||||
"+import { qux } from \"./qux\";",
|
||||
"+import { quux } from \"./quux\";",
|
||||
"+import { corge } from \"./corge\";",
|
||||
"+import { grault } from \"./grault\";",
|
||||
" ",
|
||||
" export function main() {",
|
||||
].join("\n");
|
||||
const backwardFiles = parsePatchFiles(backwardSelectionDiff).flatMap(
|
||||
(p) => p.files,
|
||||
);
|
||||
const backwardFileName = backwardFiles[0]?.name ?? "";
|
||||
|
||||
// Backward selection: start=9 > end=5 on the same side.
|
||||
// The annotation should appear at line 5 (the end point).
|
||||
export const BackwardSameSideSelection: Story = {
|
||||
args: {
|
||||
parsedFiles: backwardFiles,
|
||||
diffStyle: "unified",
|
||||
getSelectedLines: (fileName: string): SelectedLineRange | null => {
|
||||
if (fileName === backwardFileName) {
|
||||
return { start: 9, end: 5, side: "additions" };
|
||||
}
|
||||
return null;
|
||||
},
|
||||
getLineAnnotations: (fileName: string): DiffLineAnnotation<string>[] => {
|
||||
if (fileName === backwardFileName) {
|
||||
return [
|
||||
{
|
||||
lineNumber: 5,
|
||||
side: "additions",
|
||||
metadata: "active-input",
|
||||
},
|
||||
];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
renderAnnotation: () => (
|
||||
<InlinePromptInput onSubmit={fn()} onCancel={fn()} />
|
||||
),
|
||||
},
|
||||
play: expectAnnotationTextarea,
|
||||
};
|
||||
|
||||
// Cross-side selection going additions -> deletions (the
|
||||
// reverse of the typical del -> add direction).
|
||||
export const CrossSideAdditionsToDeletions: Story = {
|
||||
args: {
|
||||
parsedFiles: changeFiles,
|
||||
diffStyle: "split",
|
||||
getSelectedLines: (fileName: string): SelectedLineRange | null => {
|
||||
if (fileName === changeFileName) {
|
||||
return {
|
||||
start: 2,
|
||||
end: 3,
|
||||
side: "additions",
|
||||
endSide: "deletions",
|
||||
};
|
||||
}
|
||||
return null;
|
||||
},
|
||||
getLineAnnotations: (fileName: string): DiffLineAnnotation<string>[] => {
|
||||
if (fileName === changeFileName) {
|
||||
return [
|
||||
{
|
||||
lineNumber: 3,
|
||||
side: "deletions",
|
||||
metadata: "active-input",
|
||||
},
|
||||
];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
renderAnnotation: () => (
|
||||
<InlinePromptInput onSubmit={fn()} onCancel={fn()} />
|
||||
),
|
||||
},
|
||||
play: expectAnnotationTextarea,
|
||||
};
|
||||
|
||||
@@ -66,6 +66,7 @@ interface DiffViewerProps {
|
||||
start: number;
|
||||
end: number;
|
||||
side?: "additions" | "deletions";
|
||||
endSide?: "additions" | "deletions";
|
||||
} | null,
|
||||
) => void;
|
||||
/**
|
||||
@@ -577,6 +578,7 @@ export const DiffViewer: FC<DiffViewerProps> = ({
|
||||
start: number;
|
||||
end: number;
|
||||
side?: "additions" | "deletions";
|
||||
endSide?: "additions" | "deletions";
|
||||
} | null,
|
||||
) => onLineSelected(fileName, range)
|
||||
: () => {
|
||||
|
||||
@@ -290,6 +290,7 @@ export const GitPanel: FC<GitPanelProps> = ({
|
||||
onCommit={onCommit}
|
||||
isExpanded={isExpanded}
|
||||
diffStyle={diffStyle}
|
||||
chatInputRef={chatInputRef}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
@@ -345,7 +346,16 @@ const LocalRepoContent: FC<{
|
||||
onCommit: (repoRoot: string) => void;
|
||||
isExpanded?: boolean;
|
||||
diffStyle: DiffStyle;
|
||||
}> = ({ repoRoot, repo, diffStats, onCommit, isExpanded, diffStyle }) => {
|
||||
chatInputRef?: RefObject<ChatMessageInputRef | null>;
|
||||
}> = ({
|
||||
repoRoot,
|
||||
repo,
|
||||
diffStats,
|
||||
onCommit,
|
||||
isExpanded,
|
||||
diffStyle,
|
||||
chatInputRef,
|
||||
}) => {
|
||||
if (!repo) {
|
||||
return null;
|
||||
}
|
||||
@@ -362,6 +372,7 @@ const LocalRepoContent: FC<{
|
||||
repo={repo}
|
||||
isExpanded={isExpanded}
|
||||
diffStyle={diffStyle}
|
||||
chatInputRef={chatInputRef}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -1,18 +1,22 @@
|
||||
import { parsePatchFiles } from "@pierre/diffs";
|
||||
import type { WorkspaceAgentRepoChanges } from "api/typesGenerated";
|
||||
import { type FC, useMemo } from "react";
|
||||
import { type DiffStyle, DiffViewer } from "./DiffViewer";
|
||||
import { type FC, type RefObject, useMemo } from "react";
|
||||
import type { ChatMessageInputRef } from "./AgentChatInput";
|
||||
import { CommentableDiffViewer } from "./CommentableDiffViewer";
|
||||
import type { DiffStyle } from "./DiffViewer";
|
||||
|
||||
interface LocalDiffPanelProps {
|
||||
repo: WorkspaceAgentRepoChanges;
|
||||
isExpanded?: boolean;
|
||||
diffStyle: DiffStyle;
|
||||
chatInputRef?: RefObject<ChatMessageInputRef | null>;
|
||||
}
|
||||
|
||||
export const LocalDiffPanel: FC<LocalDiffPanelProps> = ({
|
||||
repo,
|
||||
isExpanded,
|
||||
diffStyle,
|
||||
chatInputRef,
|
||||
}) => {
|
||||
const parsedFiles = useMemo(() => {
|
||||
const diff = repo.unified_diff;
|
||||
@@ -28,11 +32,12 @@ export const LocalDiffPanel: FC<LocalDiffPanelProps> = ({
|
||||
}, [repo.unified_diff]);
|
||||
|
||||
return (
|
||||
<DiffViewer
|
||||
<CommentableDiffViewer
|
||||
parsedFiles={parsedFiles}
|
||||
isExpanded={isExpanded}
|
||||
emptyMessage="No file changes."
|
||||
diffStyle={diffStyle}
|
||||
chatInputRef={chatInputRef}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -1,15 +1,9 @@
|
||||
import type {
|
||||
DiffLineAnnotation,
|
||||
FileDiffMetadata,
|
||||
SelectedLineRange,
|
||||
} from "@pierre/diffs";
|
||||
import type { FileDiffMetadata } from "@pierre/diffs";
|
||||
import { parsePatchFiles } from "@pierre/diffs";
|
||||
import { chatDiffContents } from "api/queries/chats";
|
||||
import type * as TypesGen from "api/typesGenerated";
|
||||
import { Button } from "components/Button/Button";
|
||||
import {
|
||||
ArrowLeftIcon,
|
||||
ArrowUpIcon,
|
||||
ExternalLinkIcon,
|
||||
GitBranchIcon,
|
||||
GitMergeIcon,
|
||||
@@ -29,11 +23,13 @@ import {
|
||||
import { useQuery } from "react-query";
|
||||
import { cn } from "utils/cn";
|
||||
import type { ChatMessageInputRef } from "./AgentChatInput";
|
||||
import { CommentableDiffViewer } from "./CommentableDiffViewer";
|
||||
import { DiffStatBadge } from "./DiffStats";
|
||||
import type { DiffStyle } from "./DiffViewer";
|
||||
import { DiffViewer } from "./DiffViewer";
|
||||
import { parsePullRequestUrl } from "./pullRequest";
|
||||
|
||||
export { InlinePromptInput } from "./CommentableDiffViewer";
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Module-level counter for cache key uniqueness
|
||||
// -------------------------------------------------------------------
|
||||
@@ -46,81 +42,6 @@ import { parsePullRequestUrl } from "./pullRequest";
|
||||
*/
|
||||
let remoteDiffVersion = 0;
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Diff content extraction
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Walk the parsed hunks for a file and collect code lines that fall
|
||||
* within `startLine..endLine` on the given side. For "additions"
|
||||
* lines are matched against addition line numbers (using
|
||||
* `hunk.additionStart`); for "deletions" against deletion line
|
||||
* numbers (using `hunk.deletionStart`). Context lines that fall
|
||||
* in range are included as well.
|
||||
*/
|
||||
function extractDiffContent(
|
||||
parsedFiles: readonly FileDiffMetadata[],
|
||||
fileName: string,
|
||||
startLine: number,
|
||||
endLine: number,
|
||||
side: "additions" | "deletions",
|
||||
): string {
|
||||
const file = parsedFiles.find((f) => f.name === fileName);
|
||||
if (!file) return "";
|
||||
|
||||
const lines = side === "additions" ? file.additionLines : file.deletionLines;
|
||||
const collected: string[] = [];
|
||||
for (const hunk of file.hunks) {
|
||||
let addLine = hunk.additionStart;
|
||||
let delLine = hunk.deletionStart;
|
||||
|
||||
for (const block of hunk.hunkContent) {
|
||||
if (block.type === "context") {
|
||||
for (let i = 0; i < block.lines; i++) {
|
||||
const ln = side === "additions" ? addLine : delLine;
|
||||
if (ln >= startLine && ln <= endLine) {
|
||||
const idx =
|
||||
side === "additions"
|
||||
? block.additionLineIndex + i
|
||||
: block.deletionLineIndex + i;
|
||||
if (lines[idx] != null) collected.push(lines[idx]);
|
||||
}
|
||||
addLine++;
|
||||
delLine++;
|
||||
}
|
||||
} else {
|
||||
// ChangeContent block.
|
||||
if (side === "deletions") {
|
||||
for (let i = 0; i < block.deletions; i++) {
|
||||
if (delLine >= startLine && delLine <= endLine) {
|
||||
const line = lines[block.deletionLineIndex + i];
|
||||
if (line != null) collected.push(line);
|
||||
}
|
||||
delLine++;
|
||||
}
|
||||
// Addition lines in a change block still advance
|
||||
// the addition counter.
|
||||
addLine += block.additions;
|
||||
} else {
|
||||
// side === "additions"
|
||||
// Deletion lines in a change block still advance
|
||||
// the deletion counter.
|
||||
delLine += block.deletions;
|
||||
for (let i = 0; i < block.additions; i++) {
|
||||
if (addLine >= startLine && addLine <= endLine) {
|
||||
const line = lines[block.additionLineIndex + i];
|
||||
if (line != null) collected.push(line);
|
||||
}
|
||||
addLine++;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return collected.join("\n");
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// PR state badge
|
||||
// -------------------------------------------------------------------
|
||||
@@ -160,82 +81,6 @@ const PullRequestStateBadge: FC<{
|
||||
);
|
||||
};
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Inline prompt input
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Inline input rendered as a diff annotation under the selected
|
||||
* line(s). Supports multiline via Shift+Enter. Enter submits,
|
||||
* Escape dismisses.
|
||||
*/
|
||||
export const InlinePromptInput: FC<{
|
||||
onSubmit: (text: string) => void;
|
||||
onCancel: () => void;
|
||||
}> = ({ onSubmit, onCancel }) => {
|
||||
const [text, setText] = useState("");
|
||||
const textareaRef = useRef<HTMLTextAreaElement>(null);
|
||||
|
||||
// Focus the textarea on mount. We use a ref callback via rAF
|
||||
// rather than autoFocus because the component renders inside
|
||||
// Shadow DOM where autoFocus is unreliable.
|
||||
useEffect(() => {
|
||||
requestAnimationFrame(() => {
|
||||
textareaRef.current?.focus();
|
||||
});
|
||||
}, []);
|
||||
|
||||
return (
|
||||
<div className="px-2 py-1.5">
|
||||
<div className="rounded-lg border border-border-default/80 bg-surface-secondary/45 p-1 shadow-sm has-[textarea:focus]:ring-2 has-[textarea:focus]:ring-content-link/40">
|
||||
<textarea
|
||||
ref={textareaRef}
|
||||
className="w-full resize-none border-none bg-transparent px-3 py-2 font-sans text-sm leading-5 text-content-primary placeholder:text-content-secondary outline-none ring-0 focus:outline-none focus:ring-0"
|
||||
placeholder="Add a comment..."
|
||||
rows={2}
|
||||
value={text}
|
||||
onChange={(e) => setText(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" && !e.shiftKey) {
|
||||
e.preventDefault();
|
||||
if (text.trim()) {
|
||||
onSubmit(text.trim());
|
||||
} else {
|
||||
onCancel();
|
||||
}
|
||||
}
|
||||
if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
onCancel();
|
||||
}
|
||||
}}
|
||||
/>
|
||||
<div className="flex items-end justify-between gap-2 pl-2.5 pr-1.5 pb-1.5">
|
||||
<span className="text-xs text-content-secondary">Esc to cancel</span>
|
||||
<Button
|
||||
size="icon"
|
||||
variant="default"
|
||||
className="size-7 rounded-full transition-colors [&>svg]:!size-4 [&>svg]:p-0"
|
||||
disabled={!text.trim()}
|
||||
onMouseDown={(e: React.MouseEvent) => {
|
||||
// Prevent blur from firing before click.
|
||||
e.preventDefault();
|
||||
}}
|
||||
onClick={() => {
|
||||
if (text.trim()) {
|
||||
onSubmit(text.trim());
|
||||
}
|
||||
}}
|
||||
>
|
||||
<ArrowUpIcon />
|
||||
<span className="sr-only">Add to chat</span>
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Main component
|
||||
// -------------------------------------------------------------------
|
||||
@@ -255,16 +100,6 @@ export const RemoteDiffPanel: FC<RemoteDiffPanelProps> = ({
|
||||
diffStyle,
|
||||
diffStatus,
|
||||
}) => {
|
||||
// ---------------------------------------------------------------
|
||||
// Comment / annotation state
|
||||
// ---------------------------------------------------------------
|
||||
const [activeCommentBox, setActiveCommentBox] = useState<{
|
||||
fileName: string;
|
||||
startLine: number;
|
||||
endLine: number;
|
||||
side: "additions" | "deletions";
|
||||
} | null>(null);
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// Data fetching
|
||||
// ---------------------------------------------------------------
|
||||
@@ -283,7 +118,7 @@ export const RemoteDiffPanel: FC<RemoteDiffPanelProps> = ({
|
||||
|
||||
const parsedFiles = useMemo(() => {
|
||||
if (!diffContent) {
|
||||
return [];
|
||||
return [] as FileDiffMetadata[];
|
||||
}
|
||||
try {
|
||||
// The cacheKeyPrefix enables the worker pool's LRU cache
|
||||
@@ -302,135 +137,10 @@ export const RemoteDiffPanel: FC<RemoteDiffPanelProps> = ({
|
||||
);
|
||||
return patches.flatMap((p) => p.files);
|
||||
} catch {
|
||||
return [];
|
||||
return [] as FileDiffMetadata[];
|
||||
}
|
||||
}, [diffContent, chatId]);
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// Line interaction callbacks
|
||||
// ---------------------------------------------------------------
|
||||
const handleLineNumberClick = useCallback(
|
||||
(
|
||||
fileName: string,
|
||||
props: {
|
||||
lineNumber: number;
|
||||
annotationSide: "additions" | "deletions";
|
||||
},
|
||||
) => {
|
||||
setActiveCommentBox({
|
||||
fileName,
|
||||
startLine: props.lineNumber,
|
||||
endLine: props.lineNumber,
|
||||
side: props.annotationSide,
|
||||
});
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
const handleLineSelected = useCallback(
|
||||
(
|
||||
fileName: string,
|
||||
range: {
|
||||
start: number;
|
||||
end: number;
|
||||
side?: "additions" | "deletions";
|
||||
} | null,
|
||||
) => {
|
||||
if (!range) {
|
||||
setActiveCommentBox(null);
|
||||
return;
|
||||
}
|
||||
if (range.start === range.end) return;
|
||||
const side = range.side ?? "additions";
|
||||
setActiveCommentBox({
|
||||
fileName,
|
||||
startLine: Math.min(range.start, range.end),
|
||||
endLine: Math.max(range.start, range.end),
|
||||
side,
|
||||
});
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// Annotation helpers
|
||||
// ---------------------------------------------------------------
|
||||
const getLineAnnotations = useCallback(
|
||||
(fileName: string): DiffLineAnnotation<string>[] => {
|
||||
if (activeCommentBox && activeCommentBox.fileName === fileName) {
|
||||
return [
|
||||
{
|
||||
side: activeCommentBox.side,
|
||||
lineNumber: activeCommentBox.endLine,
|
||||
metadata: "active-input",
|
||||
},
|
||||
];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
[activeCommentBox],
|
||||
);
|
||||
|
||||
const getSelectedLines = useCallback(
|
||||
(fileName: string): SelectedLineRange | null => {
|
||||
if (activeCommentBox && activeCommentBox.fileName === fileName) {
|
||||
return {
|
||||
start: activeCommentBox.startLine,
|
||||
end: activeCommentBox.endLine,
|
||||
side: activeCommentBox.side,
|
||||
};
|
||||
}
|
||||
return null;
|
||||
},
|
||||
[activeCommentBox],
|
||||
);
|
||||
|
||||
const handleCancelComment = useCallback(() => {
|
||||
setActiveCommentBox(null);
|
||||
}, []);
|
||||
|
||||
const handleSubmitComment = useCallback(
|
||||
(text: string) => {
|
||||
if (!activeCommentBox) return;
|
||||
const content = extractDiffContent(
|
||||
parsedFiles,
|
||||
activeCommentBox.fileName,
|
||||
activeCommentBox.startLine,
|
||||
activeCommentBox.endLine,
|
||||
activeCommentBox.side,
|
||||
);
|
||||
// Single imperative call — chip inserted atomically
|
||||
// in one Lexical update. No rAF hack needed.
|
||||
chatInputRef?.current?.addFileReference({
|
||||
fileName: activeCommentBox.fileName,
|
||||
startLine: activeCommentBox.startLine,
|
||||
endLine: activeCommentBox.endLine,
|
||||
content,
|
||||
});
|
||||
if (text.trim()) {
|
||||
chatInputRef?.current?.insertText(text);
|
||||
}
|
||||
setActiveCommentBox(null);
|
||||
},
|
||||
[activeCommentBox, chatInputRef, parsedFiles],
|
||||
);
|
||||
|
||||
const renderAnnotation = useCallback(
|
||||
(annotation: DiffLineAnnotation<string>) => {
|
||||
if (annotation.metadata === "active-input") {
|
||||
if (!activeCommentBox) return null;
|
||||
return (
|
||||
<InlinePromptInput
|
||||
onSubmit={handleSubmitComment}
|
||||
onCancel={handleCancelComment}
|
||||
/>
|
||||
);
|
||||
}
|
||||
return null;
|
||||
},
|
||||
[activeCommentBox, handleSubmitComment, handleCancelComment],
|
||||
);
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// Scroll-to-file from chat input chip clicks
|
||||
// ---------------------------------------------------------------
|
||||
@@ -506,17 +216,13 @@ export const RemoteDiffPanel: FC<RemoteDiffPanelProps> = ({
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
<DiffViewer
|
||||
<CommentableDiffViewer
|
||||
parsedFiles={parsedFiles}
|
||||
isExpanded={isExpanded}
|
||||
diffStyle={diffStyle}
|
||||
isLoading={diffContentsQuery.isLoading}
|
||||
error={diffContentsQuery.isError ? diffContentsQuery.error : undefined}
|
||||
onLineNumberClick={handleLineNumberClick}
|
||||
onLineSelected={handleLineSelected}
|
||||
getLineAnnotations={getLineAnnotations}
|
||||
getSelectedLines={getSelectedLines}
|
||||
renderAnnotation={renderAnnotation}
|
||||
chatInputRef={chatInputRef}
|
||||
scrollToFile={scrollTarget}
|
||||
onScrollToFileComplete={handleScrollComplete}
|
||||
/>
|
||||
|
||||
@@ -0,0 +1,508 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
annotationLineForBox,
|
||||
annotationSideForBox,
|
||||
type CommentBoxState,
|
||||
commentBoxFromRange,
|
||||
contentRangeForBox,
|
||||
selectedLinesForBox,
|
||||
} from "./diffCommentSelection";
|
||||
|
||||
const FILE = "src/main.ts";
|
||||
|
||||
describe("commentBoxFromRange", () => {
|
||||
it("returns null when the range is null (selection cleared)", () => {
|
||||
expect(commentBoxFromRange(FILE, null)).toBeNull();
|
||||
});
|
||||
|
||||
it("ignores same-side single-line selections (handled by line number click)", () => {
|
||||
expect(
|
||||
commentBoxFromRange(FILE, {
|
||||
start: 10,
|
||||
end: 10,
|
||||
side: "additions",
|
||||
}),
|
||||
).toBe("ignore");
|
||||
});
|
||||
|
||||
it("ignores single-line selections with no side at all", () => {
|
||||
expect(commentBoxFromRange(FILE, { start: 5, end: 5 })).toBe("ignore");
|
||||
});
|
||||
|
||||
it("allows cross-side selections even when start === end", () => {
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 16,
|
||||
end: 16,
|
||||
side: "additions",
|
||||
endSide: "deletions",
|
||||
});
|
||||
expect(result).toEqual({
|
||||
fileName: FILE,
|
||||
start: 16,
|
||||
startSide: "additions",
|
||||
end: 16,
|
||||
endSide: "deletions",
|
||||
});
|
||||
});
|
||||
|
||||
it("creates a comment box for a multi-line same-side selection", () => {
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 10,
|
||||
end: 14,
|
||||
side: "deletions",
|
||||
});
|
||||
expect(result).toEqual({
|
||||
fileName: FILE,
|
||||
start: 10,
|
||||
startSide: "deletions",
|
||||
end: 14,
|
||||
endSide: "deletions",
|
||||
});
|
||||
});
|
||||
|
||||
it("creates a comment box for a multi-line cross-side selection", () => {
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 509,
|
||||
end: 219,
|
||||
side: "deletions",
|
||||
endSide: "additions",
|
||||
});
|
||||
expect(result).toEqual({
|
||||
fileName: FILE,
|
||||
start: 509,
|
||||
startSide: "deletions",
|
||||
end: 219,
|
||||
endSide: "additions",
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves raw start/end without min/max normalization", () => {
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 20,
|
||||
end: 15,
|
||||
side: "additions",
|
||||
});
|
||||
expect(result).toMatchObject({ start: 20, end: 15 });
|
||||
});
|
||||
|
||||
it("defaults side to additions when omitted", () => {
|
||||
const result = commentBoxFromRange(FILE, { start: 3, end: 7 });
|
||||
expect(result).toMatchObject({
|
||||
startSide: "additions",
|
||||
endSide: "additions",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("annotationSideForBox", () => {
|
||||
it("returns endSide (same as startSide for same-side selections)", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 1,
|
||||
startSide: "deletions",
|
||||
end: 5,
|
||||
endSide: "deletions",
|
||||
};
|
||||
expect(annotationSideForBox(box)).toBe("deletions");
|
||||
});
|
||||
|
||||
it("returns endSide for cross-side selections", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 509,
|
||||
startSide: "deletions",
|
||||
end: 219,
|
||||
endSide: "additions",
|
||||
};
|
||||
expect(annotationSideForBox(box)).toBe("additions");
|
||||
});
|
||||
});
|
||||
|
||||
describe("annotationLineForBox", () => {
|
||||
it("returns the end line number", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 509,
|
||||
startSide: "deletions",
|
||||
end: 219,
|
||||
endSide: "additions",
|
||||
};
|
||||
expect(annotationLineForBox(box)).toBe(219);
|
||||
});
|
||||
});
|
||||
|
||||
describe("selectedLinesForBox", () => {
|
||||
it("builds a range without endSide for same-side selections", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 3,
|
||||
startSide: "additions",
|
||||
end: 8,
|
||||
endSide: "additions",
|
||||
};
|
||||
const range = selectedLinesForBox(box);
|
||||
expect(range).toEqual({
|
||||
start: 3,
|
||||
end: 8,
|
||||
side: "additions",
|
||||
});
|
||||
expect(range).not.toHaveProperty("endSide");
|
||||
});
|
||||
|
||||
it("includes endSide for cross-side selections", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 509,
|
||||
startSide: "deletions",
|
||||
end: 219,
|
||||
endSide: "additions",
|
||||
};
|
||||
expect(selectedLinesForBox(box)).toEqual({
|
||||
start: 509,
|
||||
end: 219,
|
||||
side: "deletions",
|
||||
endSide: "additions",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("contentRangeForBox", () => {
|
||||
it("normalizes direction for same-side selections", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 20,
|
||||
startSide: "additions",
|
||||
end: 15,
|
||||
endSide: "additions",
|
||||
};
|
||||
expect(contentRangeForBox(box)).toEqual({
|
||||
startLine: 15,
|
||||
endLine: 20,
|
||||
side: "additions",
|
||||
});
|
||||
});
|
||||
|
||||
it("uses the end point only for cross-side selections", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 509,
|
||||
startSide: "deletions",
|
||||
end: 219,
|
||||
endSide: "additions",
|
||||
};
|
||||
expect(contentRangeForBox(box)).toEqual({
|
||||
startLine: 219,
|
||||
endLine: 219,
|
||||
side: "additions",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Edge cases
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
describe("edge cases", () => {
|
||||
// -- commentBoxFromRange ------------------------------------------
|
||||
|
||||
it("line 1 selection is not ignored (start !== end)", () => {
|
||||
// Selecting lines 1-2 on the very first line of a file.
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 1,
|
||||
end: 2,
|
||||
side: "additions",
|
||||
});
|
||||
expect(result).toMatchObject({ start: 1, end: 2 });
|
||||
});
|
||||
|
||||
it("single-line deletion-side click is ignored", () => {
|
||||
expect(
|
||||
commentBoxFromRange(FILE, {
|
||||
start: 42,
|
||||
end: 42,
|
||||
side: "deletions",
|
||||
}),
|
||||
).toBe("ignore");
|
||||
});
|
||||
|
||||
it("cross-side selection starting on additions ending on deletions", () => {
|
||||
// User drags from the right (additions) column to the left
|
||||
// (deletions) column in split view.
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 10,
|
||||
end: 12,
|
||||
side: "additions",
|
||||
endSide: "deletions",
|
||||
});
|
||||
expect(result).toEqual({
|
||||
fileName: FILE,
|
||||
start: 10,
|
||||
startSide: "additions",
|
||||
end: 12,
|
||||
endSide: "deletions",
|
||||
});
|
||||
});
|
||||
|
||||
it("very large line numbers are preserved", () => {
|
||||
// Big generated file: deletion around line 12000, addition
|
||||
// around line 9500.
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 12345,
|
||||
end: 9500,
|
||||
side: "deletions",
|
||||
endSide: "additions",
|
||||
});
|
||||
expect(result).toMatchObject({
|
||||
start: 12345,
|
||||
end: 9500,
|
||||
});
|
||||
});
|
||||
|
||||
it("endSide matching side is treated as same-side", () => {
|
||||
// Library may explicitly send endSide equal to side.
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 5,
|
||||
end: 5,
|
||||
side: "additions",
|
||||
endSide: "additions",
|
||||
});
|
||||
expect(result).toBe("ignore");
|
||||
});
|
||||
|
||||
it("backward same-side selection (start > end) is accepted", () => {
|
||||
// User shift-clicks above the anchor line.
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 100,
|
||||
end: 90,
|
||||
side: "deletions",
|
||||
});
|
||||
expect(result).toMatchObject({
|
||||
start: 100,
|
||||
startSide: "deletions",
|
||||
end: 90,
|
||||
endSide: "deletions",
|
||||
});
|
||||
});
|
||||
|
||||
it("adjacent lines (start and end differ by 1)", () => {
|
||||
const result = commentBoxFromRange(FILE, {
|
||||
start: 7,
|
||||
end: 8,
|
||||
side: "additions",
|
||||
});
|
||||
expect(result).toMatchObject({ start: 7, end: 8 });
|
||||
});
|
||||
|
||||
// -- annotationSideForBox / annotationLineForBox ------------------
|
||||
|
||||
it("annotation is placed at end for backward same-side selection", () => {
|
||||
// User selects line 50 then shift-clicks line 40.
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 50,
|
||||
startSide: "additions",
|
||||
end: 40,
|
||||
endSide: "additions",
|
||||
};
|
||||
expect(annotationLineForBox(box)).toBe(40);
|
||||
expect(annotationSideForBox(box)).toBe("additions");
|
||||
});
|
||||
|
||||
it("annotation lands on additions side for del->add cross-side", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 300,
|
||||
startSide: "deletions",
|
||||
end: 150,
|
||||
endSide: "additions",
|
||||
};
|
||||
expect(annotationSideForBox(box)).toBe("additions");
|
||||
expect(annotationLineForBox(box)).toBe(150);
|
||||
});
|
||||
|
||||
it("annotation lands on deletions side for add->del cross-side", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 10,
|
||||
startSide: "additions",
|
||||
end: 15,
|
||||
endSide: "deletions",
|
||||
};
|
||||
expect(annotationSideForBox(box)).toBe("deletions");
|
||||
expect(annotationLineForBox(box)).toBe(15);
|
||||
});
|
||||
|
||||
// -- selectedLinesForBox ------------------------------------------
|
||||
|
||||
it("backward same-side selection preserves raw order in range", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 50,
|
||||
startSide: "deletions",
|
||||
end: 40,
|
||||
endSide: "deletions",
|
||||
};
|
||||
const range = selectedLinesForBox(box);
|
||||
expect(range.start).toBe(50);
|
||||
expect(range.end).toBe(40);
|
||||
expect(range).not.toHaveProperty("endSide");
|
||||
});
|
||||
|
||||
it("cross-side with wildly different line numbers includes endSide", () => {
|
||||
// The image example: del 509, add 219.
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 509,
|
||||
startSide: "deletions",
|
||||
end: 219,
|
||||
endSide: "additions",
|
||||
};
|
||||
const range = selectedLinesForBox(box);
|
||||
expect(range).toEqual({
|
||||
start: 509,
|
||||
end: 219,
|
||||
side: "deletions",
|
||||
endSide: "additions",
|
||||
});
|
||||
});
|
||||
|
||||
// -- contentRangeForBox -------------------------------------------
|
||||
|
||||
it("same-side forward selection keeps startLine <= endLine", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 5,
|
||||
startSide: "additions",
|
||||
end: 10,
|
||||
endSide: "additions",
|
||||
};
|
||||
const { startLine, endLine } = contentRangeForBox(box);
|
||||
expect(startLine).toBe(5);
|
||||
expect(endLine).toBe(10);
|
||||
});
|
||||
|
||||
it("same-side backward selection normalizes to startLine <= endLine", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 10,
|
||||
startSide: "deletions",
|
||||
end: 5,
|
||||
endSide: "deletions",
|
||||
};
|
||||
const { startLine, endLine, side } = contentRangeForBox(box);
|
||||
expect(startLine).toBe(5);
|
||||
expect(endLine).toBe(10);
|
||||
expect(side).toBe("deletions");
|
||||
});
|
||||
|
||||
it("same-side single-line content range (line number click path)", () => {
|
||||
// This is the state created by handleLineNumberClick.
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 42,
|
||||
startSide: "additions",
|
||||
end: 42,
|
||||
endSide: "additions",
|
||||
};
|
||||
expect(contentRangeForBox(box)).toEqual({
|
||||
startLine: 42,
|
||||
endLine: 42,
|
||||
side: "additions",
|
||||
});
|
||||
});
|
||||
|
||||
it("cross-side content range never produces startLine > endLine", () => {
|
||||
// Even when the deletion line number is much larger than the
|
||||
// addition line number, the content range should be a sane
|
||||
// single-line range on one side.
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 12345,
|
||||
startSide: "deletions",
|
||||
end: 100,
|
||||
endSide: "additions",
|
||||
};
|
||||
const { startLine, endLine, side } = contentRangeForBox(box);
|
||||
expect(startLine).toBeLessThanOrEqual(endLine);
|
||||
expect(startLine).toBe(100);
|
||||
expect(endLine).toBe(100);
|
||||
expect(side).toBe("additions");
|
||||
});
|
||||
|
||||
it("cross-side add->del content range uses deletions end point", () => {
|
||||
const box: CommentBoxState = {
|
||||
fileName: FILE,
|
||||
start: 50,
|
||||
startSide: "additions",
|
||||
end: 200,
|
||||
endSide: "deletions",
|
||||
};
|
||||
expect(contentRangeForBox(box)).toEqual({
|
||||
startLine: 200,
|
||||
endLine: 200,
|
||||
side: "deletions",
|
||||
});
|
||||
});
|
||||
|
||||
// -- Round-trip: commentBoxFromRange -> helpers --------------------
|
||||
|
||||
it("round-trip: cross-side range flows correctly through all helpers", () => {
|
||||
// Simulate the full lifecycle for a cross-side selection with
|
||||
// mismatched line numbers (the original bug scenario).
|
||||
const box = commentBoxFromRange(FILE, {
|
||||
start: 509,
|
||||
end: 219,
|
||||
side: "deletions",
|
||||
endSide: "additions",
|
||||
});
|
||||
if (box === null || box === "ignore") {
|
||||
throw new Error("Expected a CommentBoxState");
|
||||
}
|
||||
|
||||
// Annotation at end point on end side.
|
||||
expect(annotationSideForBox(box)).toBe("additions");
|
||||
expect(annotationLineForBox(box)).toBe(219);
|
||||
|
||||
// Library highlight range preserves raw coordinates.
|
||||
expect(selectedLinesForBox(box)).toEqual({
|
||||
start: 509,
|
||||
end: 219,
|
||||
side: "deletions",
|
||||
endSide: "additions",
|
||||
});
|
||||
|
||||
// Content extraction uses end point only.
|
||||
const content = contentRangeForBox(box);
|
||||
expect(content.startLine).toBe(219);
|
||||
expect(content.endLine).toBe(219);
|
||||
expect(content.side).toBe("additions");
|
||||
});
|
||||
|
||||
it("round-trip: same-side backward selection flows correctly", () => {
|
||||
const box = commentBoxFromRange(FILE, {
|
||||
start: 30,
|
||||
end: 20,
|
||||
side: "deletions",
|
||||
});
|
||||
if (box === null || box === "ignore") {
|
||||
throw new Error("Expected a CommentBoxState");
|
||||
}
|
||||
|
||||
// Annotation at end (line 20).
|
||||
expect(annotationLineForBox(box)).toBe(20);
|
||||
expect(annotationSideForBox(box)).toBe("deletions");
|
||||
|
||||
// Library range preserves raw direction.
|
||||
const range = selectedLinesForBox(box);
|
||||
expect(range.start).toBe(30);
|
||||
expect(range.end).toBe(20);
|
||||
expect(range).not.toHaveProperty("endSide");
|
||||
|
||||
// Content range normalizes direction.
|
||||
expect(contentRangeForBox(box)).toEqual({
|
||||
startLine: 20,
|
||||
endLine: 30,
|
||||
side: "deletions",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,136 @@
|
||||
import type { SelectedLineRange } from "@pierre/diffs";
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Types
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
type AnnotationSide = "additions" | "deletions";
|
||||
|
||||
/**
|
||||
* Range reported by the @pierre/diffs library when a user selects
|
||||
* one or more lines. `endSide` is present only when the selection
|
||||
* crosses from one side to the other (e.g. deletions -> additions in
|
||||
* a split diff).
|
||||
*/
|
||||
interface LineSelectionRange {
|
||||
start: number;
|
||||
end: number;
|
||||
side?: AnnotationSide;
|
||||
endSide?: AnnotationSide;
|
||||
}
|
||||
|
||||
/**
|
||||
* Internal state tracked for an active inline comment input.
|
||||
*
|
||||
* `start`/`end` are the raw line numbers reported by the library,
|
||||
* each on its own side. For same-side selections both sides are
|
||||
* equal; for cross-side selections they refer to line numbers in
|
||||
* different file versions and MUST NOT be compared with min/max.
|
||||
*/
|
||||
export interface CommentBoxState {
|
||||
fileName: string;
|
||||
start: number;
|
||||
startSide: AnnotationSide;
|
||||
end: number;
|
||||
endSide: AnnotationSide;
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Pure helpers
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Compute the comment box state from a selection range reported by
|
||||
* the diff library.
|
||||
*
|
||||
* Returns:
|
||||
* - `null` when the range is null (selection cleared).
|
||||
* - `"ignore"` when the range is a same-side single-line click
|
||||
* (these are handled by `handleLineNumberClick`
|
||||
* instead).
|
||||
* - A `CommentBoxState` otherwise.
|
||||
*/
|
||||
export function commentBoxFromRange(
|
||||
fileName: string,
|
||||
range: LineSelectionRange | null,
|
||||
): CommentBoxState | null | "ignore" {
|
||||
if (!range) return null;
|
||||
|
||||
const startSide = range.side ?? "additions";
|
||||
const endSide = range.endSide ?? startSide;
|
||||
|
||||
// Single-line same-side selections are handled by the line
|
||||
// number click handler, not the range selection handler.
|
||||
if (range.start === range.end && startSide === endSide) return "ignore";
|
||||
|
||||
return {
|
||||
fileName,
|
||||
start: range.start,
|
||||
startSide,
|
||||
end: range.end,
|
||||
endSide,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The side on which the inline annotation (comment input) should be
|
||||
* rendered. For cross-side selections the annotation appears on the
|
||||
* end side so it sits visually at the bottom of the highlighted
|
||||
* range.
|
||||
*/
|
||||
export function annotationSideForBox(box: CommentBoxState): AnnotationSide {
|
||||
return box.endSide;
|
||||
}
|
||||
|
||||
/**
|
||||
* The line number at which the annotation should be placed.
|
||||
*/
|
||||
export function annotationLineForBox(box: CommentBoxState): number {
|
||||
return box.end;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the `SelectedLineRange` that the diff library needs to
|
||||
* visually highlight the selected lines. The library maps these
|
||||
* back to visual row indices internally, so we pass the raw
|
||||
* coordinates without normalization.
|
||||
*/
|
||||
export function selectedLinesForBox(box: CommentBoxState): SelectedLineRange {
|
||||
return {
|
||||
start: box.start,
|
||||
end: box.end,
|
||||
side: box.startSide,
|
||||
...(box.startSide !== box.endSide && { endSide: box.endSide }),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive a sensible single-side line range for content extraction
|
||||
* and the file reference chip.
|
||||
*
|
||||
* Same-side selections produce a normalized min..max range on that
|
||||
* side. Cross-side selections use the end point only, because start
|
||||
* and end refer to line numbers in different file versions and
|
||||
* cannot be meaningfully compared.
|
||||
*/
|
||||
export function contentRangeForBox(box: CommentBoxState): {
|
||||
startLine: number;
|
||||
endLine: number;
|
||||
side: AnnotationSide;
|
||||
} {
|
||||
if (box.startSide === box.endSide) {
|
||||
return {
|
||||
startLine: Math.min(box.start, box.end),
|
||||
endLine: Math.max(box.start, box.end),
|
||||
side: box.startSide,
|
||||
};
|
||||
}
|
||||
// Cross-side: line numbers belong to different file versions.
|
||||
// Use the end point (where the annotation appears) as the
|
||||
// reference so the file chip is meaningful.
|
||||
return {
|
||||
startLine: box.end,
|
||||
endLine: box.end,
|
||||
side: box.endSide,
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,172 @@
|
||||
import { parsePatchFiles } from "@pierre/diffs";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { extractDiffContent } from "./CommentableDiffViewer";
|
||||
|
||||
function parse(diffStr: string) {
|
||||
return parsePatchFiles(diffStr).flatMap((p) => p.files);
|
||||
}
|
||||
|
||||
/** Filter blank strings that arise from trailing newlines in parsed lines. */
|
||||
function contentLines(text: string): string[] {
|
||||
return text.split("\n").filter((l) => l.length > 0);
|
||||
}
|
||||
|
||||
// Simple diff: one context line, one changed line, one context line.
|
||||
const simpleDiff = [
|
||||
"diff --git a/app.ts b/app.ts",
|
||||
"index 1111111..2222222 100644",
|
||||
"--- a/app.ts",
|
||||
"+++ b/app.ts",
|
||||
"@@ -1,3 +1,3 @@",
|
||||
" const x = 1;",
|
||||
"-const y = 2;",
|
||||
"+const y = 42;",
|
||||
" const z = 3;",
|
||||
].join("\n");
|
||||
|
||||
// Addition-only diff: no deletions.
|
||||
const additionOnlyDiff = [
|
||||
"diff --git a/add.ts b/add.ts",
|
||||
"index 1111111..2222222 100644",
|
||||
"--- a/add.ts",
|
||||
"+++ b/add.ts",
|
||||
"@@ -1,2 +1,4 @@",
|
||||
" first;",
|
||||
"+added1;",
|
||||
"+added2;",
|
||||
" last;",
|
||||
].join("\n");
|
||||
|
||||
// Deletion-only diff: no additions.
|
||||
const deletionOnlyDiff = [
|
||||
"diff --git a/del.ts b/del.ts",
|
||||
"index 1111111..2222222 100644",
|
||||
"--- a/del.ts",
|
||||
"+++ b/del.ts",
|
||||
"@@ -1,4 +1,2 @@",
|
||||
" first;",
|
||||
"-removed1;",
|
||||
"-removed2;",
|
||||
" last;",
|
||||
].join("\n");
|
||||
|
||||
// Multi-hunk diff.
|
||||
const multiHunkDiff = [
|
||||
"diff --git a/multi.ts b/multi.ts",
|
||||
"index 1111111..2222222 100644",
|
||||
"--- a/multi.ts",
|
||||
"+++ b/multi.ts",
|
||||
"@@ -1,3 +1,3 @@",
|
||||
" aaa;",
|
||||
"-bbb;",
|
||||
"+BBB;",
|
||||
" ccc;",
|
||||
"@@ -10,3 +10,3 @@",
|
||||
" xxx;",
|
||||
"-yyy;",
|
||||
"+YYY;",
|
||||
" zzz;",
|
||||
].join("\n");
|
||||
|
||||
describe("extractDiffContent", () => {
|
||||
it("extracts addition lines from a simple change block", () => {
|
||||
const files = parse(simpleDiff);
|
||||
const result = extractDiffContent(files, "app.ts", 2, 2, "additions");
|
||||
expect(result).toContain("const y = 42;");
|
||||
expect(result).not.toContain("const y = 2;");
|
||||
});
|
||||
|
||||
it("extracts deletion lines from a simple change block", () => {
|
||||
const files = parse(simpleDiff);
|
||||
const result = extractDiffContent(files, "app.ts", 2, 2, "deletions");
|
||||
expect(result).toContain("const y = 2;");
|
||||
expect(result).not.toContain("const y = 42;");
|
||||
});
|
||||
|
||||
it("extracts a single line when startLine === endLine", () => {
|
||||
const files = parse(simpleDiff);
|
||||
// Line 1 is a context line on both sides.
|
||||
const result = extractDiffContent(files, "app.ts", 1, 1, "additions");
|
||||
expect(result).toContain("const x = 1;");
|
||||
expect(contentLines(result)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("extracts lines spanning context and change blocks", () => {
|
||||
const files = parse(simpleDiff);
|
||||
// Lines 1-3 on the addition side: context, addition, context.
|
||||
const result = extractDiffContent(files, "app.ts", 1, 3, "additions");
|
||||
const lines = contentLines(result);
|
||||
expect(lines).toHaveLength(3);
|
||||
expect(lines[0]).toContain("const x = 1;");
|
||||
expect(lines[1]).toContain("const y = 42;");
|
||||
expect(lines[2]).toContain("const z = 3;");
|
||||
});
|
||||
|
||||
it("returns empty string when range does not match any lines", () => {
|
||||
const files = parse(simpleDiff);
|
||||
const result = extractDiffContent(files, "app.ts", 100, 200, "additions");
|
||||
expect(result).toBe("");
|
||||
});
|
||||
|
||||
it("returns empty string for a non-existent file name", () => {
|
||||
const files = parse(simpleDiff);
|
||||
const result = extractDiffContent(files, "nope.ts", 1, 10, "additions");
|
||||
expect(result).toBe("");
|
||||
});
|
||||
|
||||
it("extracts additions from an addition-only hunk", () => {
|
||||
const files = parse(additionOnlyDiff);
|
||||
const result = extractDiffContent(files, "add.ts", 2, 3, "additions");
|
||||
const lines = contentLines(result);
|
||||
expect(lines).toHaveLength(2);
|
||||
expect(lines[0]).toContain("added1;");
|
||||
expect(lines[1]).toContain("added2;");
|
||||
});
|
||||
|
||||
it("returns empty for deletions side on an addition-only hunk", () => {
|
||||
const files = parse(additionOnlyDiff);
|
||||
// The deletion side has no changed lines, so the added content
|
||||
// should never appear when asking for deletions.
|
||||
const result = extractDiffContent(files, "add.ts", 2, 3, "deletions");
|
||||
expect(result).not.toContain("added1");
|
||||
expect(result).not.toContain("added2");
|
||||
});
|
||||
|
||||
it("extracts deletions from a deletion-only hunk", () => {
|
||||
const files = parse(deletionOnlyDiff);
|
||||
const result = extractDiffContent(files, "del.ts", 2, 3, "deletions");
|
||||
const lines = contentLines(result);
|
||||
expect(lines).toHaveLength(2);
|
||||
expect(lines[0]).toContain("removed1;");
|
||||
expect(lines[1]).toContain("removed2;");
|
||||
});
|
||||
|
||||
it("returns empty for additions side on a deletion-only hunk", () => {
|
||||
const files = parse(deletionOnlyDiff);
|
||||
// The addition side has no changed lines, so deleted content
|
||||
// should never appear when asking for additions.
|
||||
const result = extractDiffContent(files, "del.ts", 2, 3, "additions");
|
||||
expect(result).not.toContain("removed1");
|
||||
expect(result).not.toContain("removed2");
|
||||
});
|
||||
|
||||
it("extracts from the second hunk of a multi-hunk file", () => {
|
||||
const files = parse(multiHunkDiff);
|
||||
// Second hunk: addition side starts at line 10.
|
||||
const result = extractDiffContent(files, "multi.ts", 11, 11, "additions");
|
||||
expect(result).toContain("YYY;");
|
||||
expect(contentLines(result)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("includes context lines when they fall in the requested range", () => {
|
||||
const files = parse(multiHunkDiff);
|
||||
// Second hunk on the addition side: lines 10 (context "xxx;"),
|
||||
// 11 (addition "YYY;"), 12 (context "zzz;").
|
||||
const result = extractDiffContent(files, "multi.ts", 10, 12, "additions");
|
||||
const lines = contentLines(result);
|
||||
expect(lines).toHaveLength(3);
|
||||
expect(lines[0]).toContain("xxx;");
|
||||
expect(lines[1]).toContain("YYY;");
|
||||
expect(lines[2]).toContain("zzz;");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user