Merge pull request #151 from RooVetGit/strip_line_numbers_from_write_to_file

Strip line numbers from write to file

Author: mrubens

CHANGELOG.md

@@ -1,8 +1,8 @@
 # Roo Cline Changelog
 
-## [2.2.14]
+## [2.2.14 - 2.2.15]
 
--   Make diff editing more robust to transient errors
+-   Make diff editing more robust to transient errors / fix bugs
 
 ## [2.2.13]
 

package-lock.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.14",
+  "version": "2.2.15",
   "icon": "assets/icons/rocket.png",
   "galleryBanner": {
     "color": "#617A91",

package.json

@@ -12,7 +12,7 @@ import { ApiHandler, buildApiHandler } from "../api"
 import { ApiStream } from "../api/transform/stream"
 import { DiffViewProvider } from "../integrations/editor/DiffViewProvider"
 import { findToolName, formatContentBlockToMarkdown } from "../integrations/misc/export-markdown"
-import { extractTextFromFile, addLineNumbers } from "../integrations/misc/extract-text"
+import { extractTextFromFile, addLineNumbers, stripLineNumbers, everyLineHasLineNumbers } from "../integrations/misc/extract-text"
 import { TerminalManager } from "../integrations/terminal/TerminalManager"
 import { UrlContentFetcher } from "../services/browser/UrlContentFetcher"
 import { listFiles } from "../services/glob/list-files"
@@ -1090,7 +1090,7 @@ export class Cline {
 									await this.diffViewProvider.open(relPath)
 								}
 								// editor is open, stream content in
-								await this.diffViewProvider.update(newContent, false)
+								await this.diffViewProvider.update(everyLineHasLineNumbers(newContent) ? stripLineNumbers(newContent) : newContent, false)
 								break
 							} else {
 								if (!relPath) {
@@ -1116,7 +1116,7 @@ export class Cline {
 									await this.ask("tool", partialMessage, true).catch(() => {}) // sending true for partial even though it's not a partial, this shows the edit row before the content is streamed into the editor
 									await this.diffViewProvider.open(relPath)
 								}
-								await this.diffViewProvider.update(newContent, true)
+								await this.diffViewProvider.update(everyLineHasLineNumbers(newContent) ? stripLineNumbers(newContent) : newContent, true)
 								await delay(300) // wait for diff view to update
 								this.diffViewProvider.scrollToFirstDiff()
 

src/core/Cline.ts

@@ -1054,6 +1054,23 @@ function sum(a, b) {
                 expect(result.content).toBe('function sum(a, b) {\n    return a + b + 1;\n}')
             }
         })
+
+        it('should not exact match empty lines', () => {
+            const originalContent = 'function sum(a, b) {\n\n    return a + b;\n}'
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function sum(a, b) {
+=======
+import { a } from "a";
+function sum(a, b) {
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result.success).toBe(true)
+            if (result.success) {
+                expect(result.content).toBe('import { a } from "a";\nfunction sum(a, b) {\n\n    return a + b;\n}')
+            }
+        })
     })
 
     describe('line-constrained search', () => {

src/core/diff/strategies/__tests__/search-replace.test.ts

@@ -1,5 +1,5 @@
 import { DiffStrategy, DiffResult } from "../types"
-import { addLineNumbers } from "../../../integrations/misc/extract-text"
+import { addLineNumbers, everyLineHasLineNumbers, stripLineNumbers } from "../../../integrations/misc/extract-text"
 
 const BUFFER_LINES = 20; // Number of extra context lines to show before and after matches
 
@@ -33,7 +33,7 @@ function levenshteinDistance(a: string, b: string): number {
 }
 
 function getSimilarity(original: string, search: string): number {
-    if (original === '' || search === '') {
+    if (search === '') {
         return 1;
     }
 
@@ -140,16 +140,7 @@ Your search/replace content here
         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 => /^\s*\d+\s+\|(?!\|)/.test(line));
-        };
-
-        if (hasLineNumbers(searchContent) && hasLineNumbers(replaceContent)) {
-            const stripLineNumbers = (content: string) => {
-                return content.replace(/^\s*\d+\s+\|(?!\|)/gm, '');
-            };
-
+        if (everyLineHasLineNumbers(searchContent) && everyLineHasLineNumbers(replaceContent)) {
             searchContent = stripLineNumbers(searchContent);
             replaceContent = stripLineNumbers(replaceContent);
         }

src/core/diff/strategies/search-replace.ts

@@ -1,4 +1,4 @@
-import { addLineNumbers } from '../extract-text';
+import { addLineNumbers, everyLineHasLineNumbers, stripLineNumbers } from '../extract-text';
 
 describe('addLineNumbers', () => {
     it('should add line numbers starting from 1 by default', () => {
@@ -29,4 +29,81 @@ describe('addLineNumbers', () => {
         const expected = ' 99 | line 1\n100 | line 2';
         expect(addLineNumbers(input, 99)).toBe(expected);
     });
+});
+
+describe('everyLineHasLineNumbers', () => {
+    it('should return true for content with line numbers', () => {
+        const input = '1 | line one\n2 | line two\n3 | line three';
+        expect(everyLineHasLineNumbers(input)).toBe(true);
+    });
+
+    it('should return true for content with padded line numbers', () => {
+        const input = '  1 | line one\n  2 | line two\n  3 | line three';
+        expect(everyLineHasLineNumbers(input)).toBe(true);
+    });
+
+    it('should return false for content without line numbers', () => {
+        const input = 'line one\nline two\nline three';
+        expect(everyLineHasLineNumbers(input)).toBe(false);
+    });
+
+    it('should return false for mixed content', () => {
+        const input = '1 | line one\nline two\n3 | line three';
+        expect(everyLineHasLineNumbers(input)).toBe(false);
+    });
+
+    it('should handle empty content', () => {
+        expect(everyLineHasLineNumbers('')).toBe(false);
+    });
+
+    it('should return false for content with pipe but no line numbers', () => {
+        const input = 'a | b\nc | d';
+        expect(everyLineHasLineNumbers(input)).toBe(false);
+    });
+});
+
+describe('stripLineNumbers', () => {
+    it('should strip line numbers from content', () => {
+        const input = '1 | line one\n2 | line two\n3 | line three';
+        const expected = 'line one\nline two\nline three';
+        expect(stripLineNumbers(input)).toBe(expected);
+    });
+
+    it('should strip padded line numbers', () => {
+        const input = '  1 | line one\n  2 | line two\n  3 | line three';
+        const expected = 'line one\nline two\nline three';
+        expect(stripLineNumbers(input)).toBe(expected);
+    });
+
+    it('should handle content without line numbers', () => {
+        const input = 'line one\nline two\nline three';
+        expect(stripLineNumbers(input)).toBe(input);
+    });
+
+    it('should handle empty content', () => {
+        expect(stripLineNumbers('')).toBe('');
+    });
+
+    it('should preserve content with pipe but no line numbers', () => {
+        const input = 'a | b\nc | d';
+        expect(stripLineNumbers(input)).toBe(input);
+    });
+
+    it('should handle windows-style line endings', () => {
+        const input = '1 | line one\r\n2 | line two\r\n3 | line three';
+        const expected = 'line one\r\nline two\r\nline three';
+        expect(stripLineNumbers(input)).toBe(expected);
+    });
+
+    it('should handle content with varying line number widths', () => {
+        const input = '  1 | line one\n 10 | line two\n100 | line three';
+        const expected = 'line one\nline two\nline three';
+        expect(stripLineNumbers(input)).toBe(expected);
+    });
+
+    it('should preserve indentation after line numbers', () => {
+        const input = '1 |     indented line\n2 |   another indented';
+        const expected = '    indented line\n  another indented';
+        expect(stripLineNumbers(input)).toBe(expected);
+    });
 });

src/integrations/misc/__tests__/extract-text.test.ts

@@ -53,6 +53,7 @@ async function extractTextFromIPYNB(filePath: string): Promise<string> {
 
 	return addLineNumbers(extractedText)
 }
+
 export function addLineNumbers(content: string, startLine: number = 1): string {
 	const lines = content.split('\n')
 	const maxLineNumberWidth = String(startLine + lines.length - 1).length
@@ -61,4 +62,29 @@ export function addLineNumbers(content: string, startLine: number = 1): string {
 			const lineNumber = String(startLine + index).padStart(maxLineNumberWidth, ' ')
 			return `${lineNumber} | ${line}`
 		}).join('\n')
+}
+// Checks if every line in the content has line numbers prefixed (e.g., "1 | content" or "123 | content")
+// Line numbers must be followed by a single pipe character (not double pipes)
+export function everyLineHasLineNumbers(content: string): boolean {
+	const lines = content.split(/\r?\n/)
+	return lines.length > 0 && lines.every(line => /^\s*\d+\s+\|(?!\|)/.test(line))
+}
+
+// Strips line numbers from content while preserving the actual content
+// Handles formats like "1 | content", " 12 | content", "123 | content"
+// Preserves content that naturally starts with pipe characters
+export function stripLineNumbers(content: string): string {
+	// Split into lines to handle each line individually
+	const lines = content.split(/\r?\n/)
+	
+	// Process each line
+	const processedLines = lines.map(line => {
+		// Match line number pattern and capture everything after the pipe
+		const match = line.match(/^\s*\d+\s+\|(?!\|)\s?(.*)$/)
+		return match ? match[1] : line
+	})
+	
+	// Join back with original line endings
+	const lineEnding = content.includes('\r\n') ? '\r\n' : '\n'
+	return processedLines.join(lineEnding)
 }