From 9255dc5df903ab0e7c749926e1bea3819c0c1aa3 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Tue, 1 Apr 2025 14:33:27 -0600 Subject: [PATCH] fix(diff): Handle line number stripping for deletions in apply_diff (#2193) The multi-search-replace diff strategy previously did not correctly strip line numbers (`number | `) from the SEARCH block when the REPLACE block was empty. This occurred because the condition for stripping required both blocks to consistently have line numbers. This prevented successful deletion operations when the SEARCH block content was copied from `read_file` output (which includes line numbers) and the REPLACE block was empty. This commit updates the line number stripping condition in `applyDiff` to also trigger if the SEARCH block has line numbers and the REPLACE block is empty or contains only whitespace, resolving the bug. Additionally, a new test case has been added to `multi-search-replace.test.ts` to specifically verify this deletion scenario. All tests now pass with this updated logic. --- .../__tests__/multi-search-replace.test.ts | 17 +++++++++++++++++ .../diff/strategies/multi-search-replace.ts | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace.test.ts b/src/core/diff/strategies/__tests__/multi-search-replace.test.ts index e4a52af6d4..60d9f0508b 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace.test.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace.test.ts @@ -1486,6 +1486,23 @@ function five() { }`) } }) + + it("should delete a line when search block has line number prefix and replace is empty", async () => { + const originalContent = "line 1\nline to delete\nline 3" + const diffContent = ` +<<<<<<< SEARCH +:start_line:2 +:end_line:2 +------- +2 | line to delete +======= +>>>>>>> REPLACE` + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe("line 1\nline 3") + } + }) }) describe("insertion", () => { diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index bf80f09052..b4c0920843 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -301,7 +301,10 @@ Only use a single line of '=======' between search and replacement content, beca replaceContent = this.unescapeMarkers(replaceContent) // Strip line numbers from search and replace content if every line starts with a line number - if (everyLineHasLineNumbers(searchContent) && everyLineHasLineNumbers(replaceContent)) { + if ( + (everyLineHasLineNumbers(searchContent) && everyLineHasLineNumbers(replaceContent)) || + (everyLineHasLineNumbers(searchContent) && replaceContent.trim() === "") + ) { searchContent = stripLineNumbers(searchContent) replaceContent = stripLineNumbers(replaceContent) }