Browse Source

Add a search-and-replace diff strategy (#57)

Matt Rubens 1 year ago
parent
commit
6e1f6833b8

+ 8 - 0
CHANGELOG.md

@@ -1,5 +1,13 @@
 # Roo Cline Changelog
 
+## [2.1.17]
+
+- Switch to search/replace diffs in experimental diff editing mode
+
+## [2.1.16]
+
+- Allow copying prompts from the history screen
+
 ## [2.1.15]
 
 - Incorporate dbasclpy's [PR](https://github.com/RooVetGit/Roo-Cline/pull/54) to add support for gemini-exp-1206

+ 2 - 1
README.md

@@ -4,11 +4,12 @@ A fork of Cline, an autonomous coding agent, with some added experimental config
 - Auto-approval capabilities for commands, write, and browser operations
 - Support for .clinerules per-project custom instructions
 - Ability to run side-by-side with Cline
-- Code is unit-tested
+- Unit test coverage (written almost entirely by Roo Cline!)
 - Support for playing sound effects
 - Support for OpenRouter compression
 - Support for editing through diffs (very experimental)
 - Support for gemini-exp-1206
+- Support for copying prompts from the history screen
 
 Here's an example of Roo-Cline autonomously creating a snake game with "Always approve write operations" and "Always approve browser actions" turned on:
 

+ 2 - 2
package-lock.json

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

+ 1 - 1
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.1.16",
+  "version": "2.1.17",
   "icon": "assets/icons/rocket.png",
   "galleryBanner": {
     "color": "#617A91",

+ 4 - 4
src/core/diff/DiffStrategy.ts

@@ -1,16 +1,16 @@
 import type { DiffStrategy } from './types'
 import { UnifiedDiffStrategy } from './strategies/unified'
-
+import { SearchReplaceDiffStrategy } from './strategies/search-replace'
 /**
  * Get the appropriate diff strategy for the given model
  * @param model The name of the model being used (e.g., 'gpt-4', 'claude-3-opus')
  * @returns The appropriate diff strategy for the model
  */
 export function getDiffStrategy(model: string): DiffStrategy {
-    // For now, return UnifiedDiffStrategy for all models
+    // For now, return SearchReplaceDiffStrategy for all models
     // This architecture allows for future optimizations based on model capabilities
-    return new UnifiedDiffStrategy()
+    return new SearchReplaceDiffStrategy()
 }
 
 export type { DiffStrategy }
-export { UnifiedDiffStrategy }
+export { UnifiedDiffStrategy, SearchReplaceDiffStrategy }

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

@@ -0,0 +1,504 @@
+import { SearchReplaceDiffStrategy } from '../search-replace'
+
+describe('SearchReplaceDiffStrategy', () => {
+    let strategy: SearchReplaceDiffStrategy
+
+    beforeEach(() => {
+        strategy = new SearchReplaceDiffStrategy()
+    })
+
+    describe('applyDiff', () => {
+        it('should replace matching content', () => {
+            const originalContent = `function hello() {
+    console.log("hello")
+}
+`
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function hello() {
+    console.log("hello")
+}
+=======
+function hello() {
+    console.log("hello world")
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe(`function hello() {
+    console.log("hello world")
+}
+`)
+        })
+
+        it('should handle extra whitespace in search/replace blocks', () => {
+            const originalContent = `function test() {
+    return true;
+}
+`
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+
+function test() {
+    return true;
+}
+
+=======
+function test() {
+    return false;
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe(`function test() {
+    return false;
+}
+`)
+        })
+
+        it('should match content with different surrounding whitespace', () => {
+            const originalContent = `
+function example() {
+    return 42;
+}
+
+`
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function example() {
+    return 42;
+}
+=======
+function example() {
+    return 43;
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe(`
+function example() {
+    return 43;
+}
+
+`)
+        })
+
+        it('should match content with different indentation in search block', () => {
+            const originalContent = `    function test() {
+        return true;
+    }
+`
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function test() {
+    return true;
+}
+=======
+function test() {
+    return false;
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe(`    function test() {
+        return false;
+    }
+`)
+        })
+
+        it('should handle tab-based indentation', () => {
+            const originalContent = "function test() {\n\treturn true;\n}\n"
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function test() {
+\treturn true;
+}
+=======
+function test() {
+\treturn false;
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe("function test() {\n\treturn false;\n}\n")
+        })
+
+        it('should preserve mixed tabs and spaces', () => {
+            const originalContent = "\tclass Example {\n\t    constructor() {\n\t\tthis.value = 0;\n\t    }\n\t}"
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+\tclass Example {
+\t    constructor() {
+\t\tthis.value = 0;
+\t    }
+\t}
+=======
+\tclass Example {
+\t    constructor() {
+\t\tthis.value = 1;
+\t    }
+\t}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe("\tclass Example {\n\t    constructor() {\n\t\tthis.value = 1;\n\t    }\n\t}")
+        })
+
+        it('should handle additional indentation with tabs', () => {
+            const originalContent = "\tfunction test() {\n\t\treturn true;\n\t}"
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function test() {
+\treturn true;
+}
+=======
+function test() {
+\t// Add comment
+\treturn false;
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe("\tfunction test() {\n\t\t// Add comment\n\t\treturn false;\n\t}")
+        })
+
+        it('should preserve exact indentation characters when adding lines', () => {
+            const originalContent = "\tfunction test() {\n\t\treturn true;\n\t}"
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+\tfunction test() {
+\t\treturn true;
+\t}
+=======
+\tfunction test() {
+\t\t// First comment
+\t\t// Second comment
+\t\treturn true;
+\t}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe("\tfunction test() {\n\t\t// First comment\n\t\t// Second comment\n\t\treturn true;\n\t}")
+        })
+
+        it('should return false if search content does not match', () => {
+            const originalContent = `function hello() {
+    console.log("hello")
+}
+`
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function hello() {
+    console.log("wrong")
+}
+=======
+function hello() {
+    console.log("hello world")
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe(false)
+        })
+
+        it('should return false if diff format is invalid', () => {
+            const originalContent = `function hello() {
+    console.log("hello")
+}
+`
+            const diffContent = `test.ts
+Invalid diff format`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe(false)
+        })
+
+        it('should handle multiple lines with proper indentation', () => {
+            const originalContent = `class Example {
+    constructor() {
+        this.value = 0
+    }
+
+    getValue() {
+        return this.value
+    }
+}
+`
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+    getValue() {
+        return this.value
+    }
+=======
+    getValue() {
+        // Add logging
+        console.log("Getting value")
+        return this.value
+    }
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe(`class Example {
+    constructor() {
+        this.value = 0
+    }
+
+    getValue() {
+        // Add logging
+        console.log("Getting value")
+        return this.value
+    }
+}
+`)
+        })
+
+        it('should preserve whitespace exactly in the output', () => {
+            const originalContent = "    indented\n        more indented\n    back\n"
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+    indented
+        more indented
+    back
+=======
+    modified
+        still indented
+    end
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            expect(result).toBe("    modified\n        still indented\n    end\n")
+        })
+
+        it('should handle complex refactoring with multiple functions', () => {
+            const originalContent = `export async function extractTextFromFile(filePath: string): Promise<string> {
+	try {
+		await fs.access(filePath)
+	} catch (error) {
+		throw new Error(\`File not found: \${filePath}\`)
+	}
+	const fileExtension = path.extname(filePath).toLowerCase()
+	switch (fileExtension) {
+		case ".pdf":
+			return extractTextFromPDF(filePath)
+		case ".docx":
+			return extractTextFromDOCX(filePath)
+		case ".ipynb":
+			return extractTextFromIPYNB(filePath)
+		default:
+			const isBinary = await isBinaryFile(filePath).catch(() => false)
+			if (!isBinary) {
+				return addLineNumbers(await fs.readFile(filePath, "utf8"))
+			} else {
+				throw new Error(\`Cannot read text for file type: \${fileExtension}\`)
+			}
+	}
+}
+
+export function addLineNumbers(content: string): string {
+	const lines = content.split('\\n')
+	const maxLineNumberWidth = String(lines.length).length
+	return lines
+		.map((line, index) => {
+			const lineNumber = String(index + 1).padStart(maxLineNumberWidth, ' ')
+			return \`\${lineNumber} | \${line}\`
+		}).join('\\n')
+}`
+
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+export async function extractTextFromFile(filePath: string): Promise<string> {
+	try {
+		await fs.access(filePath)
+	} catch (error) {
+		throw new Error(\`File not found: \${filePath}\`)
+	}
+	const fileExtension = path.extname(filePath).toLowerCase()
+	switch (fileExtension) {
+		case ".pdf":
+			return extractTextFromPDF(filePath)
+		case ".docx":
+			return extractTextFromDOCX(filePath)
+		case ".ipynb":
+			return extractTextFromIPYNB(filePath)
+		default:
+			const isBinary = await isBinaryFile(filePath).catch(() => false)
+			if (!isBinary) {
+				return addLineNumbers(await fs.readFile(filePath, "utf8"))
+			} else {
+				throw new Error(\`Cannot read text for file type: \${fileExtension}\`)
+			}
+	}
+}
+
+export function addLineNumbers(content: string): string {
+	const lines = content.split('\\n')
+	const maxLineNumberWidth = String(lines.length).length
+	return lines
+		.map((line, index) => {
+			const lineNumber = String(index + 1).padStart(maxLineNumberWidth, ' ')
+			return \`\${lineNumber} | \${line}\`
+		}).join('\\n')
+}
+=======
+function extractLineRange(content: string, startLine?: number, endLine?: number): string {
+	const lines = content.split('\\n')
+	const start = startLine ? Math.max(1, startLine) : 1
+	const end = endLine ? Math.min(lines.length, endLine) : lines.length
+	
+	if (start > end || start > lines.length) {
+		throw new Error(\`Invalid line range: start=\${start}, end=\${end}, total lines=\${lines.length}\`)
+	}
+	
+	return lines.slice(start - 1, end).join('\\n')
+}
+
+export async function extractTextFromFile(filePath: string, startLine?: number, endLine?: number): Promise<string> {
+	try {
+		await fs.access(filePath)
+	} catch (error) {
+		throw new Error(\`File not found: \${filePath}\`)
+	}
+	const fileExtension = path.extname(filePath).toLowerCase()
+	let content: string
+	
+	switch (fileExtension) {
+		case ".pdf": {
+			const dataBuffer = await fs.readFile(filePath)
+			const data = await pdf(dataBuffer)
+			content = extractLineRange(data.text, startLine, endLine)
+			break
+		}
+		case ".docx": {
+			const result = await mammoth.extractRawText({ path: filePath })
+			content = extractLineRange(result.value, startLine, endLine)
+			break
+		}
+		case ".ipynb": {
+			const data = await fs.readFile(filePath, "utf8")
+			const notebook = JSON.parse(data)
+			let extractedText = ""
+			
+			for (const cell of notebook.cells) {
+				if ((cell.cell_type === "markdown" || cell.cell_type === "code") && cell.source) {
+					extractedText += cell.source.join("\\n") + "\\n"
+				}
+			}
+			content = extractLineRange(extractedText, startLine, endLine)
+			break
+		}
+		default: {
+			const isBinary = await isBinaryFile(filePath).catch(() => false)
+			if (!isBinary) {
+				const fileContent = await fs.readFile(filePath, "utf8")
+				content = extractLineRange(fileContent, startLine, endLine)
+			} else {
+				throw new Error(\`Cannot read text for file type: \${fileExtension}\`)
+			}
+		}
+	}
+	
+	return addLineNumbers(content, startLine)
+}
+
+export function addLineNumbers(content: string, startLine: number = 1): string {
+	const lines = content.split('\\n')
+	const maxLineNumberWidth = String(startLine + lines.length - 1).length
+	return lines
+		.map((line, index) => {
+			const lineNumber = String(startLine + index).padStart(maxLineNumberWidth, ' ')
+			return \`\${lineNumber} | \${line}\`
+		}).join('\\n')
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent)
+            const expected = `function extractLineRange(content: string, startLine?: number, endLine?: number): string {
+	const lines = content.split('\\n')
+	const start = startLine ? Math.max(1, startLine) : 1
+	const end = endLine ? Math.min(lines.length, endLine) : lines.length
+	
+	if (start > end || start > lines.length) {
+		throw new Error(\`Invalid line range: start=\${start}, end=\${end}, total lines=\${lines.length}\`)
+	}
+	
+	return lines.slice(start - 1, end).join('\\n')
+}
+
+export async function extractTextFromFile(filePath: string, startLine?: number, endLine?: number): Promise<string> {
+	try {
+		await fs.access(filePath)
+	} catch (error) {
+		throw new Error(\`File not found: \${filePath}\`)
+	}
+	const fileExtension = path.extname(filePath).toLowerCase()
+	let content: string
+	
+	switch (fileExtension) {
+		case ".pdf": {
+			const dataBuffer = await fs.readFile(filePath)
+			const data = await pdf(dataBuffer)
+			content = extractLineRange(data.text, startLine, endLine)
+			break
+		}
+		case ".docx": {
+			const result = await mammoth.extractRawText({ path: filePath })
+			content = extractLineRange(result.value, startLine, endLine)
+			break
+		}
+		case ".ipynb": {
+			const data = await fs.readFile(filePath, "utf8")
+			const notebook = JSON.parse(data)
+			let extractedText = ""
+			
+			for (const cell of notebook.cells) {
+				if ((cell.cell_type === "markdown" || cell.cell_type === "code") && cell.source) {
+					extractedText += cell.source.join("\\n") + "\\n"
+				}
+			}
+			content = extractLineRange(extractedText, startLine, endLine)
+			break
+		}
+		default: {
+			const isBinary = await isBinaryFile(filePath).catch(() => false)
+			if (!isBinary) {
+				const fileContent = await fs.readFile(filePath, "utf8")
+				content = extractLineRange(fileContent, startLine, endLine)
+			} else {
+				throw new Error(\`Cannot read text for file type: \${fileExtension}\`)
+			}
+		}
+	}
+	
+	return addLineNumbers(content, startLine)
+}
+
+export function addLineNumbers(content: string, startLine: number = 1): string {
+	const lines = content.split('\\n')
+	const maxLineNumberWidth = String(startLine + lines.length - 1).length
+	return lines
+		.map((line, index) => {
+			const lineNumber = String(startLine + index).padStart(maxLineNumberWidth, ' ')
+			return \`\${lineNumber} | \${line}\`
+		}).join('\\n')
+}`
+            expect(result).toBe(expected)
+        })
+    })
+
+    describe('getToolDescription', () => {
+        it('should include the current working directory', () => {
+            const cwd = '/test/dir'
+            const description = strategy.getToolDescription(cwd)
+            expect(description).toContain(`relative to the current working directory ${cwd}`)
+        })
+
+        it('should include required format elements', () => {
+            const description = strategy.getToolDescription('/test')
+            expect(description).toContain('<<<<<<< SEARCH')
+            expect(description).toContain('=======')
+            expect(description).toContain('>>>>>>> REPLACE')
+            expect(description).toContain('<apply_diff>')
+            expect(description).toContain('</apply_diff>')
+        })
+    })
+})

+ 171 - 0
src/core/diff/strategies/search-replace.ts

@@ -0,0 +1,171 @@
+import { DiffStrategy } from "../types"
+
+export class SearchReplaceDiffStrategy implements DiffStrategy {
+    getToolDescription(cwd: string): string {
+        return `## apply_diff
+Description: Request to replace existing code using search and replace blocks.
+This tool allows for precise, surgical replaces to files by specifying exactly what content to search for and what to replace it with.
+Only use this tool when you need to replace/fix existing code.
+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.
+
+Parameters:
+- path: (required) The path of the file to modify (relative to the current working directory ${cwd})
+- diff: (required) The search/replace blocks defining the changes.
+
+Format:
+1. First line must be the file path
+2. Followed by search/replace blocks:
+    \`\`\`
+    <<<<<<< SEARCH
+    [exact content to find including whitespace]
+    =======
+    [new content to replace with]
+    >>>>>>> REPLACE
+    \`\`\`
+
+Example:
+
+Original file:
+\`\`\`
+def calculate_total(items):
+    total = 0
+    for item in items:
+        total += item
+    return total
+\`\`\`
+
+Search/Replace content:
+\`\`\`
+main.py
+<<<<<<< SEARCH
+def calculate_total(items):
+    total = 0
+    for item in items:
+        total += item
+    return total
+=======
+def calculate_total(items):
+    """Calculate total with 10% markup"""
+    return sum(item * 1.1 for item in items)
+>>>>>>> REPLACE
+\`\`\`
+
+Usage:
+<apply_diff>
+<path>File path here</path>
+<diff>
+Your search/replace content here
+</diff>
+</apply_diff>`
+    }
+
+    applyDiff(originalContent: string, diffContent: string): string | false {
+        // Extract the search and replace blocks
+        const match = diffContent.match(/<<<<<<< SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n>>>>>>> REPLACE/);
+        if (!match) {
+            return false;
+        }
+
+        const [_, searchContent, replaceContent] = match;
+        
+        // Split content into lines
+        const searchLines = searchContent.trim().split('\n');
+        const replaceLines = replaceContent.trim().split('\n');
+        const originalLines = originalContent.split('\n');
+        
+        // Find the search content in the original
+        let matchIndex = -1;
+        
+        for (let i = 0; i <= originalLines.length - searchLines.length; i++) {
+            let found = true;
+            
+            for (let j = 0; j < searchLines.length; j++) {
+                const originalLine = originalLines[i + j];
+                const searchLine = searchLines[j];
+                
+                // Compare lines after removing leading/trailing whitespace
+                if (originalLine.trim() !== searchLine.trim()) {
+                    found = false;
+                    break;
+                }
+            }
+            
+            if (found) {
+                matchIndex = i;
+                break;
+            }
+        }
+        
+        if (matchIndex === -1) {
+            return false;
+        }
+        
+        // Get the matched lines from the original content
+        const matchedLines = originalLines.slice(matchIndex, matchIndex + searchLines.length);
+        
+        // Get the exact indentation (preserving tabs/spaces) of each line
+        const originalIndents = matchedLines.map(line => {
+            const match = line.match(/^[\t ]*/);
+            return match ? match[0] : '';
+        });
+        
+        // Get the exact indentation of each line in the search block
+        const searchIndents = searchLines.map(line => {
+            const match = line.match(/^[\t ]*/);
+            return match ? match[0] : '';
+        });
+        
+        // Apply the replacement while preserving exact indentation
+        const indentedReplace = replaceLines.map((line, i) => {
+            // Get the corresponding original and search indentations
+            const originalIndent = originalIndents[Math.min(i, originalIndents.length - 1)];
+            const searchIndent = searchIndents[Math.min(i, searchIndents.length - 1)];
+            
+            // Get the current line's indentation
+            const currentIndentMatch = line.match(/^[\t ]*/);
+            const currentIndent = currentIndentMatch ? currentIndentMatch[0] : '';
+            
+            // If this line has the same indentation level as the search block,
+            // use the original indentation. Otherwise, calculate the difference
+            // and preserve the exact type of whitespace characters
+            if (currentIndent.length === searchIndent.length) {
+                return originalIndent + line.trim();
+            } else {
+                // Get the corresponding search line's indentation
+                const searchLineIndex = Math.min(i, searchLines.length - 1);
+                const searchLineIndent = searchIndents[searchLineIndex];
+
+                // Get the corresponding original line's indentation
+                const originalLineIndex = Math.min(i, originalIndents.length - 1);
+                const originalLineIndent = originalIndents[originalLineIndex];
+
+                // If this line has the same indentation as its corresponding search line,
+                // use the original indentation
+                if (currentIndent === searchLineIndent) {
+                    return originalLineIndent + line.trim();
+                }
+
+                // Otherwise, preserve the original indentation structure
+                const indentChar = originalLineIndent.charAt(0) || '\t';
+                const indentLevel = Math.floor(originalLineIndent.length / indentChar.length);
+
+                // Calculate the relative indentation from the search line
+                const searchLevel = Math.floor(searchLineIndent.length / indentChar.length);
+                const currentLevel = Math.floor(currentIndent.length / indentChar.length);
+                const relativeLevel = currentLevel - searchLevel;
+
+                // Apply the relative indentation to the original level
+                const targetLevel = Math.max(0, indentLevel + relativeLevel);
+                return indentChar.repeat(targetLevel) + line.trim();
+            }
+        });
+        
+        // Construct the final content
+        const beforeMatch = originalLines.slice(0, matchIndex);
+        const afterMatch = originalLines.slice(matchIndex + searchLines.length);
+        
+        return [...beforeMatch, ...indentedReplace, ...afterMatch].join('\n');
+    }
+}