mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
refactor(site): use diff library for inline tool diffs (#22423)
Replaces the hand-rolled LCS diffing in `buildEditDiff` and the manual patch-string assembly in `buildWriteFileDiff` with [`Diff.createPatch()`](https://www.npmjs.com/package/diff) from the `diff` npm package. Both functions now just call `Diff.createPatch()` and feed the result straight into `parsePatchFiles()`, removing all the manual line splitting, prefix tagging, hunk-header arithmetic, and trailing-newline cleanup. ### Changes - Add `diff` as a dependency - `buildWriteFileDiff`: replaced ~20 lines of manual patch assembly with a single `Diff.createPatch()` call - `buildEditDiff`: replaced ~60 lines (line splitting, `Diff.diffLines` → prefixed strings, hunk counting) with a `Diff.createPatch()` call per edit - Removed the `chunkLines` helper and the `diffLines` wrapper + its test block Net: +21 / -157 lines across source and tests.
This commit is contained in:
@@ -83,6 +83,7 @@
|
||||
"cron-parser": "4.9.0",
|
||||
"cronstrue": "2.59.0",
|
||||
"dayjs": "1.11.19",
|
||||
"diff": "8.0.3",
|
||||
"emoji-mart": "5.6.0",
|
||||
"file-saver": "2.0.5",
|
||||
"formik": "2.4.9",
|
||||
|
||||
Generated
+3
@@ -163,6 +163,9 @@ importers:
|
||||
dayjs:
|
||||
specifier: 1.11.19
|
||||
version: 1.11.19
|
||||
diff:
|
||||
specifier: 8.0.3
|
||||
version: 8.0.3
|
||||
emoji-mart:
|
||||
specifier: 5.6.0
|
||||
version: 5.6.0
|
||||
|
||||
@@ -468,3 +468,178 @@ export const TaskNameGenericRendering: Story = {
|
||||
expect(canvas.queryByRole("link", { name: "View agent" })).toBeNull();
|
||||
},
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// WriteFile stories
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export const WriteFileRunning: Story = {
|
||||
args: {
|
||||
name: "write_file",
|
||||
status: "running",
|
||||
args: {
|
||||
path: "src/utils/helpers.ts",
|
||||
content:
|
||||
"export function greet(name: string): string {\n return `Hello, ${name}!`;\n}\n",
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(canvas.getByText(/Writing helpers\.ts/)).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const WriteFileSuccess: Story = {
|
||||
args: {
|
||||
name: "write_file",
|
||||
status: "completed",
|
||||
args: {
|
||||
path: "src/utils/helpers.ts",
|
||||
content:
|
||||
"export function greet(name: string): string {\n return `Hello, ${name}!`;\n}\n",
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(canvas.getByText(/Wrote helpers\.ts/)).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// EditFiles stories
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
export const EditFilesSingleRunning: Story = {
|
||||
args: {
|
||||
name: "edit_files",
|
||||
status: "running",
|
||||
args: {
|
||||
files: [
|
||||
{
|
||||
path: "src/config.ts",
|
||||
edits: [
|
||||
{
|
||||
search: "const timeout = 30;",
|
||||
replace: "const timeout = 60;",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(canvas.getByText(/Editing config\.ts/)).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const EditFilesSingleSuccess: Story = {
|
||||
args: {
|
||||
name: "edit_files",
|
||||
status: "completed",
|
||||
args: {
|
||||
files: [
|
||||
{
|
||||
path: "src/config.ts",
|
||||
edits: [
|
||||
{
|
||||
search: "const timeout = 30;",
|
||||
replace: "const timeout = 60;",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(canvas.getByText(/Edited config\.ts/)).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const EditFilesMultipleSuccess: Story = {
|
||||
args: {
|
||||
name: "edit_files",
|
||||
status: "completed",
|
||||
args: {
|
||||
files: [
|
||||
{
|
||||
path: "src/config.ts",
|
||||
edits: [
|
||||
{
|
||||
search: "const timeout = 30;",
|
||||
replace: "const timeout = 60;",
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
path: "src/server.ts",
|
||||
edits: [
|
||||
{
|
||||
search: 'const host = "localhost";',
|
||||
replace: 'const host = "0.0.0.0";',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(canvas.getByText(/Edited 2 files/)).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Exercises the LCS-based interleaved diff: only the first and last
|
||||
* lines change while the middle line stays the same, so the viewer
|
||||
* should show context around the modifications instead of removing
|
||||
* everything then re-adding everything.
|
||||
*/
|
||||
export const EditFilesInterleavedContext: Story = {
|
||||
args: {
|
||||
name: "edit_files",
|
||||
status: "completed",
|
||||
args: {
|
||||
files: [
|
||||
{
|
||||
path: "src/constants.ts",
|
||||
edits: [
|
||||
{
|
||||
search:
|
||||
'const API_URL = "http://localhost:3000";\nconst RETRY_COUNT = 3;\nconst TIMEOUT_MS = 5000;',
|
||||
replace:
|
||||
'const API_URL = "https://api.prod.example.com";\nconst RETRY_COUNT = 3;\nconst TIMEOUT_MS = 10000;',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(canvas.getByText(/Edited constants\.ts/)).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const EditFilesError: Story = {
|
||||
args: {
|
||||
name: "edit_files",
|
||||
status: "error",
|
||||
isError: true,
|
||||
args: {
|
||||
files: [
|
||||
{
|
||||
path: "src/missing.ts",
|
||||
edits: [
|
||||
{
|
||||
search: "old",
|
||||
replace: "new",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
result: { error: "File not found" },
|
||||
},
|
||||
};
|
||||
|
||||
@@ -553,6 +553,21 @@ describe("buildEditDiff", () => {
|
||||
]);
|
||||
expect(diff).not.toBeNull();
|
||||
});
|
||||
|
||||
it("preserves unchanged lines as context in hunks", () => {
|
||||
const diff = buildEditDiff("file.ts", [
|
||||
{
|
||||
search: "const x = 1;\nconst y = 2;\nconst z = 3;",
|
||||
replace: "const x = 10;\nconst y = 2;\nconst z = 30;",
|
||||
},
|
||||
]);
|
||||
expect(diff).not.toBeNull();
|
||||
const hunk = diff!.hunks[0];
|
||||
// The hunk should contain context blocks for the unchanged
|
||||
// middle line rather than removing and re-adding everything.
|
||||
const hasContext = hunk.hunkContent.some((c) => c.type === "context");
|
||||
expect(hasContext).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("constants", () => {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { FileDiffMetadata } from "@pierre/diffs";
|
||||
import { parsePatchFiles } from "@pierre/diffs";
|
||||
import * as Diff from "diff";
|
||||
import type React from "react";
|
||||
import { asRecord, asString } from "../runtimeTypeUtils";
|
||||
|
||||
@@ -256,29 +257,10 @@ export const buildWriteFileDiff = (
|
||||
path: string,
|
||||
content: string,
|
||||
): FileDiffMetadata | null => {
|
||||
const lines = content.split("\n");
|
||||
// Remove trailing empty line produced by a final newline.
|
||||
if (lines.length > 0 && lines[lines.length - 1] === "") {
|
||||
lines.pop();
|
||||
}
|
||||
if (lines.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const patchLines = [
|
||||
`diff --git a/${path} b/${path}`,
|
||||
"new file mode 100644",
|
||||
"--- /dev/null",
|
||||
`+++ b/${path}`,
|
||||
`@@ -0,0 +1,${lines.length} @@`,
|
||||
...lines.map((l) => `+${l}`),
|
||||
];
|
||||
const patch = `${patchLines.join("\n")}\n`;
|
||||
|
||||
if (!content) return null;
|
||||
const patch = Diff.createPatch(path, "", content, "", "");
|
||||
const parsed = parsePatchFiles(patch);
|
||||
if (!parsed.length || !parsed[0].files.length) {
|
||||
return null;
|
||||
}
|
||||
if (!parsed.length || !parsed[0].files.length) return null;
|
||||
return parsed[0].files[0];
|
||||
};
|
||||
|
||||
@@ -331,9 +313,9 @@ export const parseEditFilesArgs = (args: unknown): EditFilesFileEntry[] => {
|
||||
|
||||
/**
|
||||
* Builds a synthetic unified diff from search/replace edit pairs
|
||||
* for a single file. Each pair becomes a separate hunk in the
|
||||
* diff. Line numbers are synthetic since we don't have the full
|
||||
* file content.
|
||||
* for a single file. Each edit becomes a separate
|
||||
* `Diff.createPatch` call; the patches are concatenated and
|
||||
* parsed into a single FileDiffMetadata.
|
||||
*/
|
||||
export const buildEditDiff = (
|
||||
path: string,
|
||||
@@ -345,41 +327,21 @@ export const buildEditDiff = (
|
||||
// produce a double-slash that confuses the diff parser.
|
||||
const diffPath = path.startsWith("/") ? path.slice(1) : path;
|
||||
|
||||
const patchLines: string[] = [
|
||||
`diff --git a/${diffPath} b/${diffPath}`,
|
||||
`--- a/${diffPath}`,
|
||||
`+++ b/${diffPath}`,
|
||||
];
|
||||
|
||||
let lineOffset = 1;
|
||||
const patches: string[] = [];
|
||||
for (const edit of edits) {
|
||||
if (!edit.search) continue;
|
||||
const searchLines = edit.search.split("\n");
|
||||
const replaceLines = edit.replace.split("\n");
|
||||
|
||||
// Remove trailing empty line produced by a final newline.
|
||||
if (searchLines.length > 0 && searchLines[searchLines.length - 1] === "") {
|
||||
searchLines.pop();
|
||||
}
|
||||
if (
|
||||
replaceLines.length > 0 &&
|
||||
replaceLines[replaceLines.length - 1] === ""
|
||||
) {
|
||||
replaceLines.pop();
|
||||
}
|
||||
if (searchLines.length === 0 && replaceLines.length === 0) continue;
|
||||
|
||||
patchLines.push(
|
||||
`@@ -${lineOffset},${searchLines.length} +${lineOffset},${replaceLines.length} @@`,
|
||||
patches.push(Diff.createPatch(diffPath, edit.search, edit.replace, "", ""));
|
||||
}
|
||||
if (!patches.length) {
|
||||
// All edits were skipped (empty search). Produce a
|
||||
// header-only patch so the parser still returns a file
|
||||
// entry with zero hunks.
|
||||
patches.push(
|
||||
`Index: ${diffPath}\n===================================================================\n--- ${diffPath}\n+++ ${diffPath}\n`,
|
||||
);
|
||||
for (const l of searchLines) patchLines.push(`-${l}`);
|
||||
for (const l of replaceLines) patchLines.push(`+${l}`);
|
||||
|
||||
lineOffset += Math.max(searchLines.length, replaceLines.length) + 1;
|
||||
}
|
||||
|
||||
const patch = `${patchLines.join("\n")}\n`;
|
||||
const parsed = parsePatchFiles(patch);
|
||||
const parsed = parsePatchFiles(patches.join(""));
|
||||
if (!parsed.length || !parsed[0].files.length) return null;
|
||||
return parsed[0].files[0];
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user