From 4b27264a21f4213106e37985e50c84ba687dc2be Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 15 Dec 2024 22:38:21 -0500 Subject: [PATCH 1/2] Revert "Merge pull request #125 from RooVetGit/search_replace_prompt_hotfix" This reverts commit 7d7a8563b7bd466e6a9183900e9707df9c07add8, reversing changes made to 868f4cddc9435d748ab090c6f6258495ea95667f. --- CHANGELOG.md | 4 ---- package-lock.json | 4 ++-- package.json | 2 +- src/core/diff/strategies/search-replace.ts | 1 - 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd73b4f..44844ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,5 @@ # Roo Cline Changelog -## [2.2.9] - -- Fix a bug where Gemini was including line numbers in the search/replace content - ## [2.2.8] - More work on diff editing (better matching, indentation, logging) diff --git a/package-lock.json b/package-lock.json index 744ce85..733c114 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.9", + "version": "2.2.8", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.9", + "version": "2.2.8", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index 7d9d6d3..62c46fa 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.9", + "version": "2.2.8", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts index f9de1fe..21309c6 100644 --- a/src/core/diff/strategies/search-replace.ts +++ b/src/core/diff/strategies/search-replace.ts @@ -62,7 +62,6 @@ The tool will maintain proper indentation and formatting while making changes. Only a single operation is allowed per tool use. The SEARCH section must exactly match existing content including whitespace and indentation. If you're not confident in the exact content to search for, use the read_file tool first to get the exact content. -IMPORTANT: The read_file tool returns the file content with line numbers prepended to each line. However, DO NOT include line numbers in the SEARCH and REPLACE sections of the diff content. Parameters: - path: (required) The path of the file to modify (relative to the current working directory ${cwd}) From e6c79eff80c8ff0d9f51f2dddfb8b7a7fd66e841 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 15 Dec 2024 22:38:49 -0500 Subject: [PATCH 2/2] Instead of trusting the prompt, just strip line numbers from diff contents --- CHANGELOG.md | 10 +- package-lock.json | 4 +- package.json | 2 +- .../__tests__/search-replace.test.ts | 147 ++++++++++++++++++ src/core/diff/strategies/search-replace.ts | 17 +- 5 files changed, 167 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44844ba..85023ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,17 +1,9 @@ # Roo Cline Changelog -## [2.2.8] - -- More work on diff editing (better matching, indentation, logging) - -## [2.2.7] +## [2.2.6 - 2.2.10] - More fixes to search/replace diffs -## [2.2.6] - -- Add a fuzzy match tolerance when applying diffs - ## [2.2.5] - Allow MCP servers to be enabled/disabled diff --git a/package-lock.json b/package-lock.json index 733c114..3806ccb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.8", + "version": "2.2.10", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.8", + "version": "2.2.10", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index 62c46fa..f2151db 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.8", + "version": "2.2.10", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", diff --git a/src/core/diff/strategies/__tests__/search-replace.test.ts b/src/core/diff/strategies/__tests__/search-replace.test.ts index e0b348d..8e48680 100644 --- a/src/core/diff/strategies/__tests__/search-replace.test.ts +++ b/src/core/diff/strategies/__tests__/search-replace.test.ts @@ -564,6 +564,153 @@ this.init(); }); }) + describe('line number stripping', () => { + describe('line number stripping', () => { + let strategy: SearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new SearchReplaceDiffStrategy() + }) + + it('should strip line numbers from both search and replace sections', () => { + const originalContent = 'function test() {\n return true;\n}\n' + const diffContent = `test.ts +<<<<<<< SEARCH +1 | function test() { +2 | return true; +3 | } +======= +1 | function test() { +2 | return false; +3 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('function test() {\n return false;\n}\n') + } + }) + + it('should not strip when not all lines have numbers in either section', () => { + const originalContent = 'function test() {\n return true;\n}\n' + const diffContent = `test.ts +<<<<<<< SEARCH +1 | function test() { +2 | return true; +3 | } +======= +1 | function test() { + return false; +3 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(false) + }) + + it('should preserve content that naturally starts with pipe', () => { + const originalContent = '|header|another|\n|---|---|\n|data|more|\n' + const diffContent = `test.ts +<<<<<<< SEARCH +1 | |header|another| +2 | |---|---| +3 | |data|more| +======= +1 | |header|another| +2 | |---|---| +3 | |data|updated| +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('|header|another|\n|---|---|\n|data|updated|\n') + } + }) + + it('should preserve indentation when stripping line numbers', () => { + const originalContent = ' function test() {\n return true;\n }\n' + const diffContent = `test.ts +<<<<<<< SEARCH +1 | function test() { +2 | return true; +3 | } +======= +1 | function test() { +2 | return false; +3 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(' function test() {\n return false;\n }\n') + } + }) + + it('should handle different line numbers between sections', () => { + const originalContent = 'function test() {\n return true;\n}\n' + const diffContent = `test.ts +<<<<<<< SEARCH +10 | function test() { +11 | return true; +12 | } +======= +20 | function test() { +21 | return false; +22 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('function test() {\n return false;\n}\n') + } + }) + + it('should not strip content that starts with pipe but no line number', () => { + const originalContent = '| Pipe\n|---|\n| Data\n' + const diffContent = `test.ts +<<<<<<< SEARCH +| Pipe +|---| +| Data +======= +| Pipe +|---| +| Updated +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('| Pipe\n|---|\n| Updated\n') + } + }) + + it('should handle mix of line-numbered and pipe-only content', () => { + const originalContent = '| Pipe\n|---|\n| Data\n' + const diffContent = `test.ts +<<<<<<< SEARCH +| Pipe +|---| +| Data +======= +1 | | Pipe +2 | |---| +3 | | NewData +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('1 | | Pipe\n2 | |---|\n3 | | NewData\n') + } + }) + }) + }); + describe('fuzzy matching', () => { let strategy: SearchReplaceDiffStrategy diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts index 21309c6..a950b08 100644 --- a/src/core/diff/strategies/search-replace.ts +++ b/src/core/diff/strategies/search-replace.ts @@ -131,10 +131,25 @@ Your search/replace content here }; } - const [_, searchContent, replaceContent] = match; + let [_, searchContent, replaceContent] = match; // Detect line ending from original content const lineEnding = originalContent.includes('\r\n') ? '\r\n' : '\n'; + + // Strip line numbers from search and replace content if every line starts with a line number + const hasLineNumbers = (content: string) => { + const lines = content.split(/\r?\n/); + return lines.length > 0 && lines.every(line => /^\d+\s+\|(?!\|)/.test(line)); + }; + + if (hasLineNumbers(searchContent) && hasLineNumbers(replaceContent)) { + const stripLineNumbers = (content: string) => { + return content.replace(/^\d+\s+\|(?!\|)/gm, '') + }; + + searchContent = stripLineNumbers(searchContent); + replaceContent = stripLineNumbers(replaceContent); + } // Split content into lines, handling both \n and \r\n const searchLines = searchContent.split(/\r?\n/);