Revert "Fix dollar sign replacement bug in file editing (#7703)" (#7823)

This commit is contained in:
Gal Zahavi
2025-09-05 13:34:02 -07:00
committed by GitHub
parent 20d0de74e1
commit 0e284457be
6 changed files with 23 additions and 68 deletions
+1 -2
View File
@@ -509,8 +509,7 @@ export class Task {
if (oldString === '' && !isNewFile) {
return currentContent;
}
// Use split/join to ensure replacement
return currentContent.split(oldString).join(newString);
return currentContent.replaceAll(oldString, newString);
}
async scheduleToolCalls(
-16
View File
@@ -198,22 +198,6 @@ describe('EditTool', () => {
'hello world',
);
});
it('should treat $ literally and not as replacement pattern', () => {
const current = "price is $100 and pattern end is ' '";
const oldStr = 'price is $100';
const newStr = 'price is $200';
const result = applyReplacement(current, oldStr, newStr, false);
expect(result).toBe("price is $200 and pattern end is ' '");
});
it("should treat $' literally and not as a replacement pattern", () => {
const current = 'foo';
const oldStr = 'foo';
const newStr = "bar$'baz";
const result = applyReplacement(current, oldStr, newStr, false);
expect(result).toBe("bar$'baz");
});
});
describe('validateToolParams', () => {
+1 -2
View File
@@ -51,8 +51,7 @@ export function applyReplacement(
if (oldString === '' && !isNewFile) {
return currentContent;
}
// Use split/join to ensure replacement
return currentContent.split(oldString).join(newString);
return currentContent.replaceAll(oldString, newString);
}
/**
+1 -17
View File
@@ -46,11 +46,11 @@ import {
type Mock,
} from 'vitest';
import {
applyReplacement,
SmartEditTool,
type EditToolParams,
calculateReplacement,
} from './smart-edit.js';
import { applyReplacement } from './edit.js';
import { type FileDiff, ToolConfirmationOutcome } from './tools.js';
import { ToolErrorType } from './tool-error.js';
import path from 'node:path';
@@ -169,22 +169,6 @@ describe('SmartEditTool', () => {
'hello new world new',
);
});
it('should treat $ literally and not as replacement pattern', () => {
const current = 'regex end is $ and more';
const oldStr = 'regex end is $';
const newStr = 'regex end is $ and correct';
const result = applyReplacement(current, oldStr, newStr, false);
expect(result).toBe('regex end is $ and correct and more');
});
it("should treat $' literally and not as a replacement pattern", () => {
const current = 'foo';
const oldStr = 'foo';
const newStr = "bar$'baz";
const result = applyReplacement(current, oldStr, newStr, false);
expect(result).toBe("bar$'baz");
});
});
describe('calculateReplacement', () => {
+20 -1
View File
@@ -30,7 +30,26 @@ import {
} from './modifiable-tool.js';
import { IdeClient, IDEConnectionStatus } from '../ide/ide-client.js';
import { FixLLMEditWithInstruction } from '../utils/llm-edit-fixer.js';
import { applyReplacement } from './edit.js';
export function applyReplacement(
currentContent: string | null,
oldString: string,
newString: string,
isNewFile: boolean,
): string {
if (isNewFile) {
return newString;
}
if (currentContent === null) {
// Should not happen if not a new file, but defensively return empty or newString if oldString is also empty
return oldString === '' ? newString : '';
}
// If oldString is empty and it's not a new file, do not modify the content.
if (oldString === '' && !isNewFile) {
return currentContent;
}
return currentContent.replaceAll(oldString, newString);
}
interface ReplacementContext {
params: EditToolParams;