mirror of
https://github.com/pacnpal/Roo-Code.git
synced 2025-12-20 12:21:13 -05:00
Better error messages for diffs
This commit is contained in:
@@ -1237,20 +1237,25 @@ export class Cline {
|
|||||||
const originalContent = await fs.readFile(absolutePath, "utf-8")
|
const originalContent = await fs.readFile(absolutePath, "utf-8")
|
||||||
|
|
||||||
// Apply the diff to the original content
|
// Apply the diff to the original content
|
||||||
let newContent = this.diffStrategy?.applyDiff(originalContent, diffContent) ?? false
|
const diffResult = this.diffStrategy?.applyDiff(originalContent, diffContent) ?? {
|
||||||
if (newContent === false) {
|
success: false,
|
||||||
|
error: "No diff strategy available"
|
||||||
|
}
|
||||||
|
if (!diffResult.success) {
|
||||||
this.consecutiveMistakeCount++
|
this.consecutiveMistakeCount++
|
||||||
await this.say("error", `Unable to apply diff to file - contents are out of sync: ${absolutePath}`)
|
const errorDetails = diffResult.details ? `\n\nDetails:\n${JSON.stringify(diffResult.details, null, 2)}` : ''
|
||||||
pushToolResult(`Error applying diff to file: ${absolutePath} - contents are out of sync. Try re-reading the relevant lines of the file and applying the diff again.`)
|
await this.say("error", `Unable to apply diff to file: ${absolutePath}\n${diffResult.error}${errorDetails}`)
|
||||||
|
pushToolResult(`Error applying diff to file: ${absolutePath}\n${diffResult.error}${errorDetails}`)
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
const newContent = diffResult.content
|
||||||
|
|
||||||
this.consecutiveMistakeCount = 0
|
this.consecutiveMistakeCount = 0
|
||||||
|
|
||||||
// Show diff view before asking for approval
|
// Show diff view before asking for approval
|
||||||
this.diffViewProvider.editType = "modify"
|
this.diffViewProvider.editType = "modify"
|
||||||
await this.diffViewProvider.open(relPath);
|
await this.diffViewProvider.open(relPath);
|
||||||
await this.diffViewProvider.update(newContent, true);
|
await this.diffViewProvider.update(diffResult.content, true);
|
||||||
await this.diffViewProvider.scrollToFirstDiff();
|
await this.diffViewProvider.scrollToFirstDiff();
|
||||||
|
|
||||||
const completeMessage = JSON.stringify({
|
const completeMessage = JSON.stringify({
|
||||||
|
|||||||
@@ -22,7 +22,10 @@ function hello() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe('function hello() {\n console.log("hello world")\n}\n')
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe('function hello() {\n console.log("hello world")\n}\n')
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should match content with different surrounding whitespace', () => {
|
it('should match content with different surrounding whitespace', () => {
|
||||||
@@ -39,7 +42,10 @@ function example() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe('\nfunction example() {\n return 43;\n}\n\n')
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe('\nfunction example() {\n return 43;\n}\n\n')
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should match content with different indentation in search block', () => {
|
it('should match content with different indentation in search block', () => {
|
||||||
@@ -56,7 +62,10 @@ function test() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe(' function test() {\n return false;\n }\n')
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(' function test() {\n return false;\n }\n')
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should handle tab-based indentation', () => {
|
it('should handle tab-based indentation', () => {
|
||||||
@@ -73,7 +82,10 @@ function test() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe("function test() {\n\treturn false;\n}\n")
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe("function test() {\n\treturn false;\n}\n")
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should preserve mixed tabs and spaces', () => {
|
it('should preserve mixed tabs and spaces', () => {
|
||||||
@@ -94,7 +106,10 @@ function test() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe("\tclass Example {\n\t constructor() {\n\t\tthis.value = 1;\n\t }\n\t}")
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe("\tclass Example {\n\t constructor() {\n\t\tthis.value = 1;\n\t }\n\t}")
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should handle additional indentation with tabs', () => {
|
it('should handle additional indentation with tabs', () => {
|
||||||
@@ -112,7 +127,10 @@ function test() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe("\tfunction test() {\n\t\t// Add comment\n\t\treturn false;\n\t}")
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe("\tfunction test() {\n\t\t// Add comment\n\t\treturn false;\n\t}")
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should preserve exact indentation characters when adding lines', () => {
|
it('should preserve exact indentation characters when adding lines', () => {
|
||||||
@@ -131,7 +149,10 @@ function test() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
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}")
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe("\tfunction test() {\n\t\t// First comment\n\t\t// Second comment\n\t\treturn true;\n\t}")
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should handle Windows-style CRLF line endings', () => {
|
it('should handle Windows-style CRLF line endings', () => {
|
||||||
@@ -148,7 +169,10 @@ function test() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe("function test() {\r\n return false;\r\n}\r\n")
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe("function test() {\r\n return false;\r\n}\r\n")
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should return false if search content does not match', () => {
|
it('should return false if search content does not match', () => {
|
||||||
@@ -165,7 +189,7 @@ function hello() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe(false)
|
expect(result.success).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should return false if diff format is invalid', () => {
|
it('should return false if diff format is invalid', () => {
|
||||||
@@ -173,7 +197,7 @@ function hello() {
|
|||||||
const diffContent = `test.ts\nInvalid diff format`
|
const diffContent = `test.ts\nInvalid diff format`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe(false)
|
expect(result.success).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should handle multiple lines with proper indentation', () => {
|
it('should handle multiple lines with proper indentation', () => {
|
||||||
@@ -192,7 +216,10 @@ function hello() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe('class Example {\n constructor() {\n this.value = 0\n }\n\n getValue() {\n // Add logging\n console.log("Getting value")\n return this.value\n }\n}\n')
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe('class Example {\n constructor() {\n this.value = 0\n }\n\n getValue() {\n // Add logging\n console.log("Getting value")\n return this.value\n }\n}\n')
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should preserve whitespace exactly in the output', () => {
|
it('should preserve whitespace exactly in the output', () => {
|
||||||
@@ -209,7 +236,10 @@ function hello() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe(" modified\n still indented\n end\n")
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(" modified\n still indented\n end\n")
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should preserve indentation when adding new lines after existing content', () => {
|
it('should preserve indentation when adding new lines after existing content', () => {
|
||||||
@@ -226,7 +256,10 @@ function hello() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe(' onScroll={() => updateHighlights()}\n onDragOver={(e) => {\n e.preventDefault()\n e.stopPropagation()\n }}')
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(' onScroll={() => updateHighlights()}\n onDragOver={(e) => {\n e.preventDefault()\n e.stopPropagation()\n }}')
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should handle varying indentation levels correctly', () => {
|
it('should handle varying indentation levels correctly', () => {
|
||||||
@@ -264,7 +297,9 @@ class Example {
|
|||||||
>>>>>>> REPLACE`.trim();
|
>>>>>>> REPLACE`.trim();
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent);
|
const result = strategy.applyDiff(originalContent, diffContent);
|
||||||
expect(result).toBe(`
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`
|
||||||
class Example {
|
class Example {
|
||||||
constructor() {
|
constructor() {
|
||||||
this.value = 1;
|
this.value = 1;
|
||||||
@@ -275,14 +310,15 @@ class Example {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}`.trim());
|
}`.trim());
|
||||||
});
|
}
|
||||||
|
})
|
||||||
|
|
||||||
it('should handle mixed indentation styles in the same file', () => {
|
it('should handle mixed indentation styles in the same file', () => {
|
||||||
const originalContent = `class Example {
|
const originalContent = `class Example {
|
||||||
constructor() {
|
constructor() {
|
||||||
this.value = 0;
|
this.value = 0;
|
||||||
if (true) {
|
if (true) {
|
||||||
this.init();
|
this.init();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}`.trim();
|
}`.trim();
|
||||||
@@ -305,7 +341,9 @@ class Example {
|
|||||||
>>>>>>> REPLACE`;
|
>>>>>>> REPLACE`;
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent);
|
const result = strategy.applyDiff(originalContent, diffContent);
|
||||||
expect(result).toBe(`class Example {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`class Example {
|
||||||
constructor() {
|
constructor() {
|
||||||
this.value = 1;
|
this.value = 1;
|
||||||
if (true) {
|
if (true) {
|
||||||
@@ -314,7 +352,8 @@ class Example {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}`);
|
}`);
|
||||||
});
|
}
|
||||||
|
})
|
||||||
|
|
||||||
it('should handle Python-style significant whitespace', () => {
|
it('should handle Python-style significant whitespace', () => {
|
||||||
const originalContent = `def example():
|
const originalContent = `def example():
|
||||||
@@ -338,17 +377,20 @@ class Example {
|
|||||||
>>>>>>> REPLACE`;
|
>>>>>>> REPLACE`;
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent);
|
const result = strategy.applyDiff(originalContent, diffContent);
|
||||||
expect(result).toBe(`def example():
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`def example():
|
||||||
if condition:
|
if condition:
|
||||||
do_something()
|
do_something()
|
||||||
while items:
|
while items:
|
||||||
item = items.pop()
|
item = items.pop()
|
||||||
process(item)
|
process(item)
|
||||||
return True`);
|
return True`);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should preserve empty lines with indentation', () => {
|
it('should preserve empty lines with indentation', () => {
|
||||||
const originalContent = `function test() {
|
const originalContent = `function test() {
|
||||||
const x = 1;
|
const x = 1;
|
||||||
|
|
||||||
if (x) {
|
if (x) {
|
||||||
@@ -368,7 +410,9 @@ class Example {
|
|||||||
>>>>>>> REPLACE`;
|
>>>>>>> REPLACE`;
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent);
|
const result = strategy.applyDiff(originalContent, diffContent);
|
||||||
expect(result).toBe(`function test() {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function test() {
|
||||||
const x = 1;
|
const x = 1;
|
||||||
|
|
||||||
// Check x
|
// Check x
|
||||||
@@ -376,6 +420,7 @@ class Example {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
}`);
|
}`);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle indentation when replacing entire blocks', () => {
|
it('should handle indentation when replacing entire blocks', () => {
|
||||||
@@ -406,7 +451,9 @@ class Example {
|
|||||||
>>>>>>> REPLACE`;
|
>>>>>>> REPLACE`;
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent);
|
const result = strategy.applyDiff(originalContent, diffContent);
|
||||||
expect(result).toBe(`class Test {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`class Test {
|
||||||
method() {
|
method() {
|
||||||
try {
|
try {
|
||||||
if (true) {
|
if (true) {
|
||||||
@@ -417,6 +464,7 @@ class Example {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}`);
|
}`);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle negative indentation relative to search content', () => {
|
it('should handle negative indentation relative to search content', () => {
|
||||||
@@ -438,7 +486,9 @@ class Example {
|
|||||||
>>>>>>> REPLACE`;
|
>>>>>>> REPLACE`;
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent);
|
const result = strategy.applyDiff(originalContent, diffContent);
|
||||||
expect(result).toBe(`class Example {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`class Example {
|
||||||
constructor() {
|
constructor() {
|
||||||
if (true) {
|
if (true) {
|
||||||
this.init();
|
this.init();
|
||||||
@@ -446,6 +496,7 @@ class Example {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}`);
|
}`);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle extreme negative indentation (no indent)', () => {
|
it('should handle extreme negative indentation (no indent)', () => {
|
||||||
@@ -464,13 +515,16 @@ this.init();
|
|||||||
>>>>>>> REPLACE`;
|
>>>>>>> REPLACE`;
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent);
|
const result = strategy.applyDiff(originalContent, diffContent);
|
||||||
expect(result).toBe(`class Example {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`class Example {
|
||||||
constructor() {
|
constructor() {
|
||||||
if (true) {
|
if (true) {
|
||||||
this.init();
|
this.init();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}`);
|
}`);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle mixed indentation changes in replace block', () => {
|
it('should handle mixed indentation changes in replace block', () => {
|
||||||
@@ -495,7 +549,9 @@ this.init();
|
|||||||
>>>>>>> REPLACE`;
|
>>>>>>> REPLACE`;
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent);
|
const result = strategy.applyDiff(originalContent, diffContent);
|
||||||
expect(result).toBe(`class Example {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`class Example {
|
||||||
constructor() {
|
constructor() {
|
||||||
if (true) {
|
if (true) {
|
||||||
this.init();
|
this.init();
|
||||||
@@ -504,6 +560,7 @@ this.init();
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}`);
|
}`);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -530,7 +587,10 @@ function getData() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe('function getData() {\n const data = fetchData();\n return data.filter(Boolean);\n}\n')
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe('function getData() {\n const data = fetchData();\n return data.filter(Boolean);\n}\n')
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should not match when content is too different (<90% similar)', () => {
|
it('should not match when content is too different (<90% similar)', () => {
|
||||||
@@ -547,7 +607,7 @@ function processData(data) {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe(false)
|
expect(result.success).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should match content with extra whitespace', () => {
|
it('should match content with extra whitespace', () => {
|
||||||
@@ -564,7 +624,10 @@ function sum(a, b) {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent)
|
const result = strategy.applyDiff(originalContent, diffContent)
|
||||||
expect(result).toBe('function sum(a, b) {\n return a + b + 1;\n}')
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe('function sum(a, b) {\n return a + b + 1;\n}')
|
||||||
|
}
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -601,7 +664,9 @@ function two() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
|
const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
|
||||||
expect(result).toBe(`function one() {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function one() {
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -612,6 +677,7 @@ function two() {
|
|||||||
function three() {
|
function three() {
|
||||||
return 3;
|
return 3;
|
||||||
}`)
|
}`)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should find and replace within buffer zone (5 lines before/after)', () => {
|
it('should find and replace within buffer zone (5 lines before/after)', () => {
|
||||||
@@ -642,7 +708,9 @@ function three() {
|
|||||||
// Even though we specify lines 5-7, it should still find the match at lines 9-11
|
// Even though we specify lines 5-7, it should still find the match at lines 9-11
|
||||||
// because it's within the 5-line buffer zone
|
// because it's within the 5-line buffer zone
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
|
const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
|
||||||
expect(result).toBe(`function one() {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function one() {
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -653,6 +721,7 @@ function two() {
|
|||||||
function three() {
|
function three() {
|
||||||
return "three";
|
return "three";
|
||||||
}`)
|
}`)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should not find matches outside search range and buffer zone', () => {
|
it('should not find matches outside search range and buffer zone', () => {
|
||||||
@@ -691,7 +760,7 @@ function five() {
|
|||||||
// Searching around function two() (lines 5-7)
|
// Searching around function two() (lines 5-7)
|
||||||
// function five() is more than 5 lines away, so it shouldn't match
|
// function five() is more than 5 lines away, so it shouldn't match
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
|
const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
|
||||||
expect(result).toBe(false)
|
expect(result.success).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should handle search range at start of file', () => {
|
it('should handle search range at start of file', () => {
|
||||||
@@ -716,13 +785,16 @@ function one() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 1, 3)
|
const result = strategy.applyDiff(originalContent, diffContent, 1, 3)
|
||||||
expect(result).toBe(`function one() {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function one() {
|
||||||
return "one";
|
return "one";
|
||||||
}
|
}
|
||||||
|
|
||||||
function two() {
|
function two() {
|
||||||
return 2;
|
return 2;
|
||||||
}`)
|
}`)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should handle search range at end of file', () => {
|
it('should handle search range at end of file', () => {
|
||||||
@@ -747,13 +819,16 @@ function two() {
|
|||||||
>>>>>>> REPLACE`
|
>>>>>>> REPLACE`
|
||||||
|
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
|
const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
|
||||||
expect(result).toBe(`function one() {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function one() {
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
function two() {
|
function two() {
|
||||||
return "two";
|
return "two";
|
||||||
}`)
|
}`)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should match specific instance of duplicate code using line numbers', () => {
|
it('should match specific instance of duplicate code using line numbers', () => {
|
||||||
@@ -790,7 +865,9 @@ function processData(data) {
|
|||||||
|
|
||||||
// Target the second instance of processData
|
// Target the second instance of processData
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 10, 12)
|
const result = strategy.applyDiff(originalContent, diffContent, 10, 12)
|
||||||
expect(result).toBe(`function processData(data) {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function processData(data) {
|
||||||
return data.map(x => x * 2);
|
return data.map(x => x * 2);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -808,6 +885,7 @@ function processData(data) {
|
|||||||
function moreStuff() {
|
function moreStuff() {
|
||||||
console.log("world");
|
console.log("world");
|
||||||
}`)
|
}`)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should search from start line to end of file when only start_line is provided', () => {
|
it('should search from start line to end of file when only start_line is provided', () => {
|
||||||
@@ -837,7 +915,9 @@ function three() {
|
|||||||
|
|
||||||
// Only provide start_line, should search from there to end of file
|
// Only provide start_line, should search from there to end of file
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 8)
|
const result = strategy.applyDiff(originalContent, diffContent, 8)
|
||||||
expect(result).toBe(`function one() {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function one() {
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -848,6 +928,7 @@ function two() {
|
|||||||
function three() {
|
function three() {
|
||||||
return "three";
|
return "three";
|
||||||
}`)
|
}`)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should search from start of file to end line when only end_line is provided', () => {
|
it('should search from start of file to end line when only end_line is provided', () => {
|
||||||
@@ -877,7 +958,9 @@ function one() {
|
|||||||
|
|
||||||
// Only provide end_line, should search from start of file to there
|
// Only provide end_line, should search from start of file to there
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, undefined, 4)
|
const result = strategy.applyDiff(originalContent, diffContent, undefined, 4)
|
||||||
expect(result).toBe(`function one() {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function one() {
|
||||||
return "one";
|
return "one";
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -888,6 +971,7 @@ function two() {
|
|||||||
function three() {
|
function three() {
|
||||||
return 3;
|
return 3;
|
||||||
}`)
|
}`)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should prioritize exact line match over expanded search', () => {
|
it('should prioritize exact line match over expanded search', () => {
|
||||||
@@ -921,7 +1005,9 @@ function process() {
|
|||||||
// Should match the second instance exactly at lines 10-12
|
// Should match the second instance exactly at lines 10-12
|
||||||
// even though the first instance at 6-8 is within the expanded search range
|
// even though the first instance at 6-8 is within the expanded search range
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 10, 12)
|
const result = strategy.applyDiff(originalContent, diffContent, 10, 12)
|
||||||
expect(result).toBe(`
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`
|
||||||
function one() {
|
function one() {
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
@@ -937,7 +1023,8 @@ function process() {
|
|||||||
function two() {
|
function two() {
|
||||||
return 2;
|
return 2;
|
||||||
}`)
|
}`)
|
||||||
})
|
}
|
||||||
|
})
|
||||||
|
|
||||||
it('should fall back to expanded search only if exact match fails', () => {
|
it('should fall back to expanded search only if exact match fails', () => {
|
||||||
const originalContent = `
|
const originalContent = `
|
||||||
@@ -966,7 +1053,9 @@ function process() {
|
|||||||
// Specify wrong line numbers (3-5), but content exists at 6-8
|
// Specify wrong line numbers (3-5), but content exists at 6-8
|
||||||
// Should still find and replace it since it's within the expanded range
|
// Should still find and replace it since it's within the expanded range
|
||||||
const result = strategy.applyDiff(originalContent, diffContent, 3, 5)
|
const result = strategy.applyDiff(originalContent, diffContent, 3, 5)
|
||||||
expect(result).toBe(`function one() {
|
expect(result.success).toBe(true)
|
||||||
|
if (result.success) {
|
||||||
|
expect(result.content).toBe(`function one() {
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -977,6 +1066,7 @@ function process() {
|
|||||||
function two() {
|
function two() {
|
||||||
return 2;
|
return 2;
|
||||||
}`)
|
}`)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { DiffStrategy } from "../types"
|
import { DiffStrategy, DiffResult } from "../types"
|
||||||
|
|
||||||
function levenshteinDistance(a: string, b: string): number {
|
function levenshteinDistance(a: string, b: string): number {
|
||||||
const matrix: number[][] = [];
|
const matrix: number[][] = [];
|
||||||
@@ -115,11 +115,20 @@ Your search/replace content here
|
|||||||
</apply_diff>`
|
</apply_diff>`
|
||||||
}
|
}
|
||||||
|
|
||||||
applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): string | false {
|
applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): DiffResult {
|
||||||
// Extract the search and replace blocks
|
// Extract the search and replace blocks
|
||||||
const match = diffContent.match(/<<<<<<< SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n>>>>>>> REPLACE/);
|
const match = diffContent.match(/<<<<<<< SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n>>>>>>> REPLACE/);
|
||||||
if (!match) {
|
if (!match) {
|
||||||
return false;
|
// Log detailed format information
|
||||||
|
console.log('Invalid Diff Format Debug:', {
|
||||||
|
expectedFormat: "<<<<<<< SEARCH\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE",
|
||||||
|
tip: "Make sure to include both SEARCH and REPLACE sections with correct markers"
|
||||||
|
});
|
||||||
|
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
error: "Invalid diff format - missing required SEARCH/REPLACE sections"
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const [_, searchContent, replaceContent] = match;
|
const [_, searchContent, replaceContent] = match;
|
||||||
@@ -135,12 +144,26 @@ Your search/replace content here
|
|||||||
// First try exact line range if provided
|
// First try exact line range if provided
|
||||||
let matchIndex = -1;
|
let matchIndex = -1;
|
||||||
let bestMatchScore = 0;
|
let bestMatchScore = 0;
|
||||||
|
let bestMatchContent = "";
|
||||||
|
|
||||||
if (startLine !== undefined && endLine !== undefined) {
|
if (startLine !== undefined && endLine !== undefined) {
|
||||||
// Convert to 0-based index
|
// Convert to 0-based index
|
||||||
const exactStartIndex = startLine - 1;
|
const exactStartIndex = startLine - 1;
|
||||||
const exactEndIndex = endLine - 1;
|
const exactEndIndex = endLine - 1;
|
||||||
|
|
||||||
|
if (exactStartIndex < 0 || exactEndIndex >= originalLines.length) {
|
||||||
|
// Log detailed debug information
|
||||||
|
console.log('Invalid Line Range Debug:', {
|
||||||
|
requestedRange: { start: startLine, end: endLine },
|
||||||
|
fileBounds: { start: 1, end: originalLines.length }
|
||||||
|
});
|
||||||
|
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
error: `Line range ${startLine}-${endLine} is invalid (file has ${originalLines.length} lines)`,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
// Check exact range first
|
// Check exact range first
|
||||||
const originalChunk = originalLines.slice(exactStartIndex, exactEndIndex + 1).join('\n');
|
const originalChunk = originalLines.slice(exactStartIndex, exactEndIndex + 1).join('\n');
|
||||||
const searchChunk = searchLines.join('\n');
|
const searchChunk = searchLines.join('\n');
|
||||||
@@ -149,6 +172,7 @@ Your search/replace content here
|
|||||||
if (similarity >= this.fuzzyThreshold) {
|
if (similarity >= this.fuzzyThreshold) {
|
||||||
matchIndex = exactStartIndex;
|
matchIndex = exactStartIndex;
|
||||||
bestMatchScore = similarity;
|
bestMatchScore = similarity;
|
||||||
|
bestMatchContent = originalChunk;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -177,13 +201,26 @@ Your search/replace content here
|
|||||||
if (similarity > bestMatchScore) {
|
if (similarity > bestMatchScore) {
|
||||||
bestMatchScore = similarity;
|
bestMatchScore = similarity;
|
||||||
matchIndex = i;
|
matchIndex = i;
|
||||||
|
bestMatchContent = originalChunk;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Require similarity to meet threshold
|
// Require similarity to meet threshold
|
||||||
if (matchIndex === -1 || bestMatchScore < this.fuzzyThreshold) {
|
if (matchIndex === -1 || bestMatchScore < this.fuzzyThreshold) {
|
||||||
return false;
|
const searchChunk = searchLines.join('\n');
|
||||||
|
// Log detailed debug information to console
|
||||||
|
console.log('Search/Replace Debug Info:', {
|
||||||
|
similarity: bestMatchScore,
|
||||||
|
threshold: this.fuzzyThreshold,
|
||||||
|
searchContent: searchChunk,
|
||||||
|
bestMatch: bestMatchContent || undefined
|
||||||
|
});
|
||||||
|
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
error: `No sufficiently similar match found${startLine !== undefined ? ` near lines ${startLine}-${endLine}` : ''} (${Math.round(bestMatchScore * 100)}% similar, needs ${Math.round(this.fuzzyThreshold * 100)}%)`
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get the matched lines from the original content
|
// Get the matched lines from the original content
|
||||||
@@ -229,6 +266,10 @@ Your search/replace content here
|
|||||||
const beforeMatch = originalLines.slice(0, matchIndex);
|
const beforeMatch = originalLines.slice(0, matchIndex);
|
||||||
const afterMatch = originalLines.slice(matchIndex + searchLines.length);
|
const afterMatch = originalLines.slice(matchIndex + searchLines.length);
|
||||||
|
|
||||||
return [...beforeMatch, ...indentedReplaceLines, ...afterMatch].join(lineEnding);
|
const finalContent = [...beforeMatch, ...indentedReplaceLines, ...afterMatch].join(lineEnding);
|
||||||
|
return {
|
||||||
|
success: true,
|
||||||
|
content: finalContent
|
||||||
|
};
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1,5 +1,5 @@
|
|||||||
import { applyPatch } from "diff"
|
import { applyPatch } from "diff"
|
||||||
import { DiffStrategy } from "../types"
|
import { DiffStrategy, DiffResult } from "../types"
|
||||||
|
|
||||||
export class UnifiedDiffStrategy implements DiffStrategy {
|
export class UnifiedDiffStrategy implements DiffStrategy {
|
||||||
getToolDescription(cwd: string): string {
|
getToolDescription(cwd: string): string {
|
||||||
@@ -108,7 +108,30 @@ Your diff here
|
|||||||
</apply_diff>`
|
</apply_diff>`
|
||||||
}
|
}
|
||||||
|
|
||||||
applyDiff(originalContent: string, diffContent: string): string | false {
|
applyDiff(originalContent: string, diffContent: string): DiffResult {
|
||||||
return applyPatch(originalContent, diffContent) as string | false
|
try {
|
||||||
|
const result = applyPatch(originalContent, diffContent)
|
||||||
|
if (result === false) {
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
error: "Failed to apply unified diff - patch rejected",
|
||||||
|
details: {
|
||||||
|
searchContent: diffContent
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
success: true,
|
||||||
|
content: result
|
||||||
|
}
|
||||||
|
} catch (error) {
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
error: `Error applying unified diff: ${error.message}`,
|
||||||
|
details: {
|
||||||
|
searchContent: diffContent
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,17 @@
|
|||||||
/**
|
/**
|
||||||
* Interface for implementing different diff strategies
|
* Interface for implementing different diff strategies
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
export type DiffResult =
|
||||||
|
| { success: true; content: string }
|
||||||
|
| { success: false; error: string; details?: {
|
||||||
|
similarity?: number;
|
||||||
|
threshold?: number;
|
||||||
|
matchedRange?: { start: number; end: number };
|
||||||
|
searchContent?: string;
|
||||||
|
bestMatch?: string;
|
||||||
|
}};
|
||||||
|
|
||||||
export interface DiffStrategy {
|
export interface DiffStrategy {
|
||||||
/**
|
/**
|
||||||
* Get the tool description for this diff strategy
|
* Get the tool description for this diff strategy
|
||||||
@@ -15,7 +26,7 @@ export interface DiffStrategy {
|
|||||||
* @param diffContent The diff content in the strategy's format
|
* @param diffContent The diff content in the strategy's format
|
||||||
* @param startLine Optional line number where the search block starts. If not provided, searches the entire file.
|
* @param startLine Optional line number where the search block starts. If not provided, searches the entire file.
|
||||||
* @param endLine Optional line number where the search block ends. If not provided, searches the entire file.
|
* @param endLine Optional line number where the search block ends. If not provided, searches the entire file.
|
||||||
* @returns The new content after applying the diff, or false if the diff could not be applied
|
* @returns A DiffResult object containing either the successful result or error details
|
||||||
*/
|
*/
|
||||||
applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): string | false
|
applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): DiffResult
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user