Skip to content

Commit fc93a2e

Browse files
committed
fix: make search and replace edit copy less confusing
1 parent ff931ec commit fc93a2e

File tree

4 files changed

+35
-27
lines changed

4 files changed

+35
-27
lines changed

gui/src/util/clientTools/findAndReplaceUtils.test.ts

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { EditOperation } from "core/tools/definitions/multiEdit";
22
import { describe, expect, it } from "vitest";
33
import {
4+
EMPTY_NON_FIRST_EDIT_MESSAGE,
5+
FOUND_MULTIPLE_FIND_STRINGS_ERROR,
46
performFindAndReplace,
57
validateCreatingForMultiEdit,
68
validateSingleEdit,
@@ -152,15 +154,15 @@ describe("performFindAndReplace", () => {
152154

153155
expect(() => {
154156
performFindAndReplace(content, "xyz", "abc");
155-
}).toThrow('String not found in file: "xyz"');
157+
}).toThrow('string not found in file: "xyz"');
156158
});
157159

158160
it("should throw error with edit context when string not found", () => {
159161
const content = "Hello world";
160162

161163
expect(() => {
162164
performFindAndReplace(content, "xyz", "abc", false, 2);
163-
}).toThrow('Edit #3: String not found in file: "xyz"');
165+
}).toThrow('edit at index 2: string not found in file: "xyz"');
164166
});
165167

166168
it("should throw error when multiple occurrences exist and replaceAll is false", () => {
@@ -169,7 +171,7 @@ describe("performFindAndReplace", () => {
169171
expect(() => {
170172
performFindAndReplace(content, "Hello", "Hi", false);
171173
}).toThrow(
172-
'String "Hello" appears 3 times in the file. Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.',
174+
`String "Hello" appears 3 times in the file. ${FOUND_MULTIPLE_FIND_STRINGS_ERROR}`,
173175
);
174176
});
175177

@@ -179,7 +181,7 @@ describe("performFindAndReplace", () => {
179181
expect(() => {
180182
performFindAndReplace(content, "test", "exam", false, 1);
181183
}).toThrow(
182-
'Edit #2: String "test" appears 3 times in the file. Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.',
184+
`edit at index 1: String "test" appears 3 times in the file. ${FOUND_MULTIPLE_FIND_STRINGS_ERROR}`,
183185
);
184186
});
185187
});
@@ -296,19 +298,21 @@ describe("validateSingleEdit", () => {
296298
it("should include edit number in error messages when index provided", () => {
297299
expect(() => {
298300
validateSingleEdit(null as any, "new", 2);
299-
}).toThrow("Edit #3: old_string is required");
301+
}).toThrow("edit at index 2: old_string is required");
300302
});
301303

302304
it("should include edit number for new_string errors", () => {
303305
expect(() => {
304306
validateSingleEdit("old", undefined as any, 0);
305-
}).toThrow("Edit #1: new_string is required");
307+
}).toThrow("edit at index 0: new_string is required");
306308
});
307309

308310
it("should include edit number for identical strings error", () => {
309311
expect(() => {
310312
validateSingleEdit("same", "same", 4);
311-
}).toThrow("Edit #5: old_string and new_string must be different");
313+
}).toThrow(
314+
"edit at index 4: old_string and new_string must be different",
315+
);
312316
});
313317

314318
it("should not include context when index not provided", () => {
@@ -382,9 +386,7 @@ describe("validateCreatingForMultiEdit", () => {
382386

383387
expect(() => {
384388
validateCreatingForMultiEdit(edits);
385-
}).toThrow(
386-
"edit #2: only the first edit can contain an empty old_string, which is only used for file creation.",
387-
);
389+
}).toThrow(`edit at index 1: ${EMPTY_NON_FIRST_EDIT_MESSAGE}`);
388390
});
389391

390392
it("should throw error with correct edit number for empty old_string", () => {
@@ -396,9 +398,7 @@ describe("validateCreatingForMultiEdit", () => {
396398

397399
expect(() => {
398400
validateCreatingForMultiEdit(edits);
399-
}).toThrow(
400-
"edit #3: only the first edit can contain an empty old_string, which is only used for file creation.",
401-
);
401+
}).toThrow(`edit at index 2: ${EMPTY_NON_FIRST_EDIT_MESSAGE}`);
402402
});
403403

404404
it("should throw error for multiple empty old_strings in subsequent edits", () => {
@@ -410,9 +410,7 @@ describe("validateCreatingForMultiEdit", () => {
410410

411411
expect(() => {
412412
validateCreatingForMultiEdit(edits);
413-
}).toThrow(
414-
"edit #2: only the first edit can contain an empty old_string, which is only used for file creation.",
415-
);
413+
}).toThrow(`edit at index 1: ${EMPTY_NON_FIRST_EDIT_MESSAGE}`);
416414
});
417415
});
418416

gui/src/util/clientTools/findAndReplaceUtils.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { EditOperation } from "core/tools/definitions/multiEdit";
22

3+
export const FOUND_MULTIPLE_FIND_STRINGS_ERROR =
4+
"Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.";
5+
36
/**
47
* Performs a find and replace operation on text content with proper handling of special characters
58
*/
@@ -10,10 +13,10 @@ export function performFindAndReplace(
1013
replaceAll: boolean = false,
1114
index?: number, // For error messages
1215
): string {
13-
const errorContext = index !== undefined ? `Edit #${index + 1}: ` : "";
16+
const errorContext = index !== undefined ? `edit at index ${index}: ` : "";
1417
// Check if old_string exists in current content
1518
if (!content.includes(oldString)) {
16-
throw new Error(`${errorContext}String not found in file: "${oldString}"`);
19+
throw new Error(`${errorContext}string not found in file: "${oldString}"`);
1720
}
1821

1922
if (replaceAll) {
@@ -35,7 +38,7 @@ export function performFindAndReplace(
3538

3639
if (count > 1) {
3740
throw new Error(
38-
`${errorContext}String "${oldString}" appears ${count} times in the file. Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.`,
41+
`${errorContext}String "${oldString}" appears ${count} times in the file. ${FOUND_MULTIPLE_FIND_STRINGS_ERROR}`,
3942
);
4043
}
4144

@@ -57,7 +60,7 @@ export function validateSingleEdit(
5760
newString: string,
5861
index?: number,
5962
): void {
60-
const context = index !== undefined ? `Edit #${index + 1}: ` : "";
63+
const context = index !== undefined ? `edit at index ${index}: ` : "";
6164

6265
if (!oldString && oldString !== "") {
6366
throw new Error(`${context}old_string is required`);
@@ -70,6 +73,8 @@ export function validateSingleEdit(
7073
}
7174
}
7275

76+
export const EMPTY_NON_FIRST_EDIT_MESSAGE =
77+
"contains empty old_string. Only the first edit can contain an empty old_string, which is only used for file creation.";
7378
export function validateCreatingForMultiEdit(edits: EditOperation[]) {
7479
const isCreating = edits[0].old_string === "";
7580
if (edits.length > 1) {
@@ -81,7 +86,7 @@ export function validateCreatingForMultiEdit(edits: EditOperation[]) {
8186
for (let i = 1; i < edits.length; i++) {
8287
if (edits[i].old_string === "") {
8388
throw new Error(
84-
`edit #${i + 1}: only the first edit can contain an empty old_string, which is only used for file creation.`,
89+
`edit at index ${i}: ${EMPTY_NON_FIRST_EDIT_MESSAGE}`,
8590
);
8691
}
8792
}

gui/src/util/clientTools/multiEditImpl.test.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as ideUtils from "core/util/ideUtils";
22
import { beforeEach, describe, expect, it, Mock, vi } from "vitest";
33
import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate";
44
import { ClientToolExtras } from "./callClientTool";
5+
import { FOUND_MULTIPLE_FIND_STRINGS_ERROR } from "./findAndReplaceUtils";
56
import { multiEditImpl } from "./multiEditImpl";
67

78
vi.mock("uuid", () => ({
@@ -72,7 +73,7 @@ describe("multiEditImpl", () => {
7273
"id",
7374
mockExtras,
7475
),
75-
).rejects.toThrow("Edit #1: old_string is required");
76+
).rejects.toThrow("edit at index 0: old_string is required");
7677
});
7778

7879
it("should throw if edit has undefined new_string", async () => {
@@ -85,7 +86,7 @@ describe("multiEditImpl", () => {
8586
"id",
8687
mockExtras,
8788
),
88-
).rejects.toThrow("Edit #1: new_string is required");
89+
).rejects.toThrow("edit at index 0: new_string is required");
8990
});
9091

9192
it("should throw if old_string equals new_string", async () => {
@@ -98,7 +99,9 @@ describe("multiEditImpl", () => {
9899
"id",
99100
mockExtras,
100101
),
101-
).rejects.toThrow("Edit #1: old_string and new_string must be different");
102+
).rejects.toThrow(
103+
"edit at index 0: old_string and new_string must be different",
104+
);
102105
});
103106
});
104107

@@ -201,7 +204,9 @@ describe("multiEditImpl", () => {
201204
"id",
202205
mockExtras,
203206
),
204-
).rejects.toThrow('Edit #2: String not found in file: "not found"');
207+
).rejects.toThrow(
208+
'edit at index 1: string not found in file: "not found"',
209+
);
205210
});
206211

207212
it("should throw if multiple occurrences without replace_all", async () => {
@@ -219,7 +224,7 @@ describe("multiEditImpl", () => {
219224
mockExtras,
220225
),
221226
).rejects.toThrow(
222-
'Edit #1: String "test" appears 3 times in the file. Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.',
227+
`edit at index 0: String "test" appears 3 times in the file. ${FOUND_MULTIPLE_FIND_STRINGS_ERROR}`,
223228
);
224229
});
225230
});

gui/src/util/clientTools/singleFindAndReplaceImpl.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ describe("singleFindAndReplaceImpl", () => {
148148

149149
await expect(
150150
singleFindAndReplaceImpl(args, "tool-call-id", mockExtras),
151-
).rejects.toThrow(`String not found in file: "not found"`);
151+
).rejects.toThrow(`string not found in file: "not found"`);
152152
});
153153

154154
it("should replace single occurrence", async () => {

0 commit comments

Comments
 (0)