Browse Source

Instead of trusting the prompt, just strip line numbers from diff contents

Matt Rubens 1 year ago
parent
commit
e6c79eff80

+ 1 - 9
CHANGELOG.md

@@ -1,17 +1,9 @@
 # Roo Cline Changelog
 # 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
 -   More fixes to search/replace diffs
 
 
-## [2.2.6]
-
--   Add a fuzzy match tolerance when applying diffs
-
 ## [2.2.5]
 ## [2.2.5]
 
 
 -   Allow MCP servers to be enabled/disabled
 -   Allow MCP servers to be enabled/disabled

+ 2 - 2
package-lock.json

@@ -1,12 +1,12 @@
 {
 {
   "name": "roo-cline",
   "name": "roo-cline",
-  "version": "2.2.8",
+  "version": "2.2.10",
   "lockfileVersion": 3,
   "lockfileVersion": 3,
   "requires": true,
   "requires": true,
   "packages": {
   "packages": {
     "": {
     "": {
       "name": "roo-cline",
       "name": "roo-cline",
-      "version": "2.2.8",
+      "version": "2.2.10",
       "dependencies": {
       "dependencies": {
         "@anthropic-ai/bedrock-sdk": "^0.10.2",
         "@anthropic-ai/bedrock-sdk": "^0.10.2",
         "@anthropic-ai/sdk": "^0.26.0",
         "@anthropic-ai/sdk": "^0.26.0",

+ 1 - 1
package.json

@@ -3,7 +3,7 @@
   "displayName": "Roo Cline",
   "displayName": "Roo Cline",
   "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.",
   "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.",
   "publisher": "RooVeterinaryInc",
   "publisher": "RooVeterinaryInc",
-  "version": "2.2.8",
+  "version": "2.2.10",
   "icon": "assets/icons/rocket.png",
   "icon": "assets/icons/rocket.png",
   "galleryBanner": {
   "galleryBanner": {
     "color": "#617A91",
     "color": "#617A91",

+ 147 - 0
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', () => {
     describe('fuzzy matching', () => {
         let strategy: SearchReplaceDiffStrategy
         let strategy: SearchReplaceDiffStrategy
 
 

+ 16 - 1
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
         // Detect line ending from original content
         const lineEnding = originalContent.includes('\r\n') ? '\r\n' : '\n';
         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
         // Split content into lines, handling both \n and \r\n
         const searchLines = searchContent.split(/\r?\n/);
         const searchLines = searchContent.split(/\r?\n/);