mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
Fix overdependence on start/end lines in diff strategy (#2567)
* Add test for issue #2556 * Fix overdependence on start/end lines in diff strategy
This commit is contained in:
Matt Rubens
GitHub
parent
45fb958505
commit
b196489b97
2 changed files with 99 additions and 135 deletions
|
|
@ -1541,7 +1541,7 @@ function five() {
|
|||
})
|
||||
})
|
||||
|
||||
describe("insertion/deletion", () => {
|
||||
describe("deletion", () => {
|
||||
let strategy: MultiSearchReplaceDiffStrategy
|
||||
|
||||
beforeEach(() => {
|
||||
|
|
@ -1646,126 +1646,6 @@ function five() {
|
|||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe("insertion", () => {
|
||||
it("should insert code at specified line when search block is empty", async () => {
|
||||
const originalContent = `function test() {
|
||||
const x = 1;
|
||||
return x;
|
||||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:2
|
||||
:end_line:2
|
||||
-------
|
||||
=======
|
||||
console.log("Adding log");
|
||||
>>>>>>> REPLACE`
|
||||
|
||||
const result = await strategy.applyDiff(originalContent, diffContent, 2, 2)
|
||||
expect(result.success).toBe(true)
|
||||
if (result.success) {
|
||||
expect(result.content).toBe(`function test() {
|
||||
console.log("Adding log");
|
||||
const x = 1;
|
||||
return x;
|
||||
}`)
|
||||
}
|
||||
})
|
||||
|
||||
it("should preserve indentation when inserting at nested location", async () => {
|
||||
const originalContent = `function test() {
|
||||
if (true) {
|
||||
const x = 1;
|
||||
}
|
||||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:3
|
||||
:end_line:3
|
||||
-------
|
||||
=======
|
||||
console.log("Before");
|
||||
console.log("After");
|
||||
>>>>>>> REPLACE`
|
||||
|
||||
const result = await strategy.applyDiff(originalContent, diffContent, 3, 3)
|
||||
expect(result.success).toBe(true)
|
||||
if (result.success) {
|
||||
expect(result.content).toBe(`function test() {
|
||||
if (true) {
|
||||
console.log("Before");
|
||||
console.log("After");
|
||||
const x = 1;
|
||||
}
|
||||
}`)
|
||||
}
|
||||
})
|
||||
|
||||
it("should handle insertion at start of file", async () => {
|
||||
const originalContent = `function test() {
|
||||
return true;
|
||||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
:end_line:1
|
||||
-------
|
||||
=======
|
||||
// Copyright 2024
|
||||
// License: MIT
|
||||
|
||||
>>>>>>> REPLACE`
|
||||
|
||||
const result = await strategy.applyDiff(originalContent, diffContent, 1, 1)
|
||||
expect(result.success).toBe(true)
|
||||
if (result.success) {
|
||||
expect(result.content).toBe(`// Copyright 2024
|
||||
// License: MIT
|
||||
|
||||
function test() {
|
||||
return true;
|
||||
}`)
|
||||
}
|
||||
})
|
||||
|
||||
it("should handle insertion at end of file", async () => {
|
||||
const originalContent = `function test() {
|
||||
return true;
|
||||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:4
|
||||
:end_line:4
|
||||
-------
|
||||
=======
|
||||
// End of file
|
||||
>>>>>>> REPLACE`
|
||||
|
||||
const result = await strategy.applyDiff(originalContent, diffContent, 4, 4)
|
||||
expect(result.success).toBe(true)
|
||||
if (result.success) {
|
||||
expect(result.content).toBe(`function test() {
|
||||
return true;
|
||||
}
|
||||
// End of file`)
|
||||
}
|
||||
})
|
||||
|
||||
it("should error if no start_line is provided for insertion", async () => {
|
||||
const originalContent = `function test() {
|
||||
return true;
|
||||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
=======
|
||||
console.log("test");
|
||||
>>>>>>> REPLACE`
|
||||
|
||||
const result = await strategy.applyDiff(originalContent, diffContent)
|
||||
expect(result.success).toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("fuzzy matching", () => {
|
||||
|
|
@ -1949,6 +1829,98 @@ function three() {
|
|||
}
|
||||
})
|
||||
|
||||
it("should work correctly on this example with line numbers that are slightly off", async () => {
|
||||
const originalContent = `.game-container {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 1rem;
|
||||
}
|
||||
|
||||
.chess-board-container {
|
||||
display: flex;
|
||||
gap: 1rem;
|
||||
align-items: center;
|
||||
}
|
||||
|
||||
.overlay {
|
||||
position: absolute;
|
||||
top: 0;
|
||||
left: 0;
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
background-color: rgba(0, 0, 0, 0.5);
|
||||
z-index: 999; /* Ensure it's above the board but below the promotion dialog */
|
||||
}
|
||||
|
||||
.game-container.promotion-active .chess-board,
|
||||
.game-container.promotion-active .game-toolbar,
|
||||
.game-container.promotion-active .game-info-container {
|
||||
filter: blur(2px);
|
||||
pointer-events: none; /* Disable clicks on these elements */
|
||||
}
|
||||
|
||||
.game-container.promotion-active .promotion-dialog {
|
||||
z-index: 1000; /* Ensure it's above the overlay */
|
||||
pointer-events: auto; /* Enable clicks on the promotion dialog */
|
||||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:12
|
||||
:end_line:13
|
||||
-------
|
||||
.overlay {
|
||||
=======
|
||||
.piece {
|
||||
will-change: transform;
|
||||
}
|
||||
|
||||
.overlay {
|
||||
>>>>>>> REPLACE
|
||||
`
|
||||
|
||||
const result = await strategy.applyDiff(originalContent, diffContent)
|
||||
expect(result.success).toBe(true)
|
||||
if (result.success) {
|
||||
expect(result.content).toBe(`.game-container {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 1rem;
|
||||
}
|
||||
|
||||
.chess-board-container {
|
||||
display: flex;
|
||||
gap: 1rem;
|
||||
align-items: center;
|
||||
}
|
||||
|
||||
.piece {
|
||||
will-change: transform;
|
||||
}
|
||||
|
||||
.overlay {
|
||||
position: absolute;
|
||||
top: 0;
|
||||
left: 0;
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
background-color: rgba(0, 0, 0, 0.5);
|
||||
z-index: 999; /* Ensure it's above the board but below the promotion dialog */
|
||||
}
|
||||
|
||||
.game-container.promotion-active .chess-board,
|
||||
.game-container.promotion-active .game-toolbar,
|
||||
.game-container.promotion-active .game-info-container {
|
||||
filter: blur(2px);
|
||||
pointer-events: none; /* Disable clicks on these elements */
|
||||
}
|
||||
|
||||
.game-container.promotion-active .promotion-dialog {
|
||||
z-index: 1000; /* Ensure it's above the overlay */
|
||||
pointer-events: auto; /* Enable clicks on the promotion dialog */
|
||||
}`)
|
||||
}
|
||||
})
|
||||
|
||||
it("should not find matches outside search range and buffer zone", async () => {
|
||||
const originalContent = `
|
||||
function one() {
|
||||
|
|
|
|||
|
|
@ -7,8 +7,9 @@ import { ToolUse } from "../../assistant-message"
|
|||
const BUFFER_LINES = 40 // Number of extra context lines to show before and after matches
|
||||
|
||||
function getSimilarity(original: string, search: string): number {
|
||||
// Empty searches are no longer supported
|
||||
if (search === "") {
|
||||
return 1
|
||||
return 0
|
||||
}
|
||||
|
||||
// Normalize strings by removing extra whitespace but preserve case
|
||||
|
|
@ -367,7 +368,6 @@ Only use a single line of '=======' between search and replacement content, beca
|
|||
const replacements = matches
|
||||
.map((match) => ({
|
||||
startLine: Number(match[2] ?? 0),
|
||||
endLine: Number(match[4] ?? resultLines.length),
|
||||
searchContent: match[6],
|
||||
replaceContent: match[7],
|
||||
}))
|
||||
|
|
@ -376,7 +376,6 @@ Only use a single line of '=======' between search and replacement content, beca
|
|||
for (const replacement of replacements) {
|
||||
let { searchContent, replaceContent } = replacement
|
||||
let startLine = replacement.startLine + (replacement.startLine === 0 ? 0 : delta)
|
||||
let endLine = replacement.endLine + delta
|
||||
|
||||
// First unescape any escaped markers in the content
|
||||
searchContent = this.unescapeMarkers(searchContent)
|
||||
|
|
@ -409,23 +408,16 @@ Only use a single line of '=======' between search and replacement content, beca
|
|||
let searchLines = searchContent === "" ? [] : searchContent.split(/\r?\n/)
|
||||
let replaceLines = replaceContent === "" ? [] : replaceContent.split(/\r?\n/)
|
||||
|
||||
// Validate that empty search requires start line
|
||||
if (searchLines.length === 0 && !startLine) {
|
||||
// Validate that search content is not empty
|
||||
if (searchLines.length === 0) {
|
||||
diffResults.push({
|
||||
success: false,
|
||||
error: `Empty search content requires start_line to be specified\n\nDebug Info:\n- Empty search content is only valid for insertions at a specific line\n- For insertions, specify the line number where content should be inserted`,
|
||||
error: `Empty search content is not allowed\n\nDebug Info:\n- Search content cannot be empty\n- For insertions, provide a specific line using :start_line: and include content to search for\n- For example, match a single line to insert before/after it`,
|
||||
})
|
||||
continue
|
||||
}
|
||||
|
||||
// Validate that empty search requires same start and end line
|
||||
if (searchLines.length === 0 && startLine && endLine && startLine !== endLine) {
|
||||
diffResults.push({
|
||||
success: false,
|
||||
error: `Empty search content requires start_line and end_line to be the same (got ${startLine}-${endLine})\n\nDebug Info:\n- Empty search content is only valid for insertions at a specific line\n- For insertions, use the same line number for both start_line and end_line`,
|
||||
})
|
||||
continue
|
||||
}
|
||||
let endLine = replacement.startLine + searchLines.length - 1
|
||||
|
||||
// Initialize search variables
|
||||
let matchIndex = -1
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue