From 6e1f6833b843f0f93a308a6b7f95399b90317629 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 10 Dec 2024 12:44:38 -0500 Subject: [PATCH] Add a search-and-replace diff strategy (#57) --- CHANGELOG.md | 8 + README.md | 3 +- package-lock.json | 4 +- package.json | 2 +- src/core/diff/DiffStrategy.ts | 8 +- .../__tests__/search-replace.test.ts | 504 ++++++++++++++++++ src/core/diff/strategies/search-replace.ts | 171 ++++++ 7 files changed, 692 insertions(+), 8 deletions(-) create mode 100644 src/core/diff/strategies/__tests__/search-replace.test.ts create mode 100644 src/core/diff/strategies/search-replace.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 209577e20..3c54636ea 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 0bda3424a..99eb5614b 100644 --- a/README.md +++ b/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: diff --git a/package-lock.json b/package-lock.json index a5b69a4a2..e1825d788 100644 --- a/package-lock.json +++ b/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", diff --git a/package.json b/package.json index 6260d48bf..12b9696d8 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.1.16", + "version": "2.1.17", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", diff --git a/src/core/diff/DiffStrategy.ts b/src/core/diff/DiffStrategy.ts index 439c719fd..6562520b7 100644 --- a/src/core/diff/DiffStrategy.ts +++ b/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 } diff --git a/src/core/diff/strategies/__tests__/search-replace.test.ts b/src/core/diff/strategies/__tests__/search-replace.test.ts new file mode 100644 index 000000000..644defc5a --- /dev/null +++ b/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 { + 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 { + 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 { + 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 { + 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('') + expect(description).toContain('') + }) + }) +}) diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts new file mode 100644 index 000000000..37bc57a64 --- /dev/null +++ b/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: + +File path here + +Your search/replace content here + +` + } + + 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'); + } +} \ No newline at end of file