Skip to content

Commit ec8ebaa

Browse files
werdnumaswinashok44
authored andcommitted
Fix newline insertion bug in replace tool (google-gemini#18595)
1 parent c7ef8fe commit ec8ebaa

2 files changed

Lines changed: 39 additions & 2 deletions

File tree

packages/core/src/tools/edit.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,43 @@ describe('EditTool', () => {
372372
expect(result.newContent).toBe(expectedContent);
373373
expect(result.occurrences).toBe(1);
374374
});
375+
376+
it('should NOT insert extra newlines when replacing a block preceded by a blank line (regression)', async () => {
377+
const content = '\n function oldFunc() {\n // some code\n }';
378+
const result = await calculateReplacement(mockConfig, {
379+
params: {
380+
file_path: 'test.js',
381+
instruction: 'test',
382+
old_string: 'function oldFunc() {\n // some code\n }', // Two spaces after function to trigger regex
383+
new_string: 'function newFunc() {\n // new code\n}', // Unindented
384+
},
385+
currentContent: content,
386+
abortSignal,
387+
});
388+
389+
// The blank line at the start should be preserved as-is,
390+
// and the discovered indentation (2 spaces) should be applied to each line.
391+
const expectedContent = '\n function newFunc() {\n // new code\n }';
392+
expect(result.newContent).toBe(expectedContent);
393+
});
394+
395+
it('should NOT insert extra newlines in flexible replacement when old_string starts with a blank line (regression)', async () => {
396+
const content = ' // some comment\n\n function oldFunc() {}';
397+
const result = await calculateReplacement(mockConfig, {
398+
params: {
399+
file_path: 'test.js',
400+
instruction: 'test',
401+
old_string: '\nfunction oldFunc() {}',
402+
new_string: '\n function newFunc() {}', // Include desired indentation
403+
},
404+
currentContent: content,
405+
abortSignal,
406+
});
407+
408+
// The blank line at the start is preserved, and the new block is inserted.
409+
const expectedContent = ' // some comment\n\n function newFunc() {}';
410+
expect(result.newContent).toBe(expectedContent);
411+
});
375412
});
376413

377414
describe('validateToolParams', () => {

packages/core/src/tools/edit.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ async function calculateFlexibleReplacement(
167167
if (isMatch) {
168168
flexibleOccurrences++;
169169
const firstLineInMatch = window[0];
170-
const indentationMatch = firstLineInMatch.match(/^(\s*)/);
170+
const indentationMatch = firstLineInMatch.match(/^([ \t]*)/);
171171
const indentation = indentationMatch ? indentationMatch[1] : '';
172172
const newBlockWithIndent = replaceLines.map(
173173
(line: string) => `${indentation}${line}`,
@@ -229,7 +229,7 @@ async function calculateRegexReplacement(
229229

230230
// The final pattern captures leading whitespace (indentation) and then matches the token pattern.
231231
// 'm' flag enables multi-line mode, so '^' matches the start of any line.
232-
const finalPattern = `^(\\s*)${pattern}`;
232+
const finalPattern = `^([ \t]*)${pattern}`;
233233
const flexibleRegex = new RegExp(finalPattern, 'm');
234234

235235
const match = flexibleRegex.exec(currentContent);

0 commit comments

Comments
 (0)