Skip to content

Commit fcf40d5

Browse files
roomoteroomote
committed
fix: preserve trailing newlines in stripLineNumbers for apply_diff
- Modified stripLineNumbers function to preserve original trailing newlines - This fixes issue #8020 where line numbers at the last line were not being stripped correctly - The regex in multi-search-replace.ts was removing trailing newlines, causing stripLineNumbers to fail - Added tests to verify the fix works for various scenarios including Windows CRLF line endings
1 parent ee58c5d commit fcf40d5

File tree

5 files changed

+265
-1
lines changed

5 files changed

+265
-1
lines changed

‎src/core/diff/strategies/__tests__/multi-search-replace-issue-8020.spec.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace"
2+
3+
describe("MultiSearchReplaceDiffStrategy - Issue #8020", () => {
4+
let strategy: MultiSearchReplaceDiffStrategy
5+
6+
beforeEach(() => {
7+
strategy = new MultiSearchReplaceDiffStrategy()
8+
})
9+
10+
it("should strip line numbers from last line without trailing newline", async () => {
11+
// This reproduces the exact issue from #8020
12+
// The problem was that the regex was removing the trailing newline,
13+
// which caused stripLineNumbers to fail on the last line
14+
const originalContent = "line 1\nline 2\nline 3"
15+
16+
// Diff content with line numbers on every line including the last
17+
const diffContent = `<<<<<<< SEARCH
18+
1 | line 1
19+
2 | line 2
20+
3 | line 3
21+
=======
22+
1 | line 1
23+
2 | modified
24+
3 | line 3
25+
>>>>>>> REPLACE`
26+
27+
const result = await strategy.applyDiff(originalContent, diffContent)
28+
29+
if (!result.success) {
30+
console.error("Failed with:", result.error || result.failParts)
31+
}
32+
33+
expect(result.success).toBe(true)
34+
if (result.success) {
35+
expect(result.content).toBe("line 1\nmodified\nline 3")
36+
}
37+
})
38+
39+
it("should handle the exact case from issue #8020", async () => {
40+
// Simplified version of the actual case
41+
const originalContent = "some code);"
42+
43+
const diffContent = `<<<<<<< SEARCH
44+
1479 | some code);
45+
=======
46+
1488 | some code);
47+
>>>>>>> REPLACE`
48+
49+
const result = await strategy.applyDiff(originalContent, diffContent)
50+
51+
if (!result.success) {
52+
console.error("Failed with:", result.error || result.failParts)
53+
}
54+
55+
expect(result.success).toBe(true)
56+
if (result.success) {
57+
// Content should remain the same since we're just replacing with the same
58+
expect(result.content).toBe("some code);")
59+
}
60+
})
61+
})

‎src/core/diff/strategies/__tests__/multi-search-replace-line-number.spec.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace"
2+
3+
describe("MultiSearchReplaceDiffStrategy - Line Number Stripping", () => {
4+
describe("line number stripping at last line", () => {
5+
let strategy: MultiSearchReplaceDiffStrategy
6+
7+
beforeEach(() => {
8+
strategy = new MultiSearchReplaceDiffStrategy()
9+
})
10+
11+
it("should correctly strip line numbers from the last line in SEARCH and REPLACE blocks", async () => {
12+
// This test case reproduces the issue from GitHub issue #8020
13+
// where the last line with a line number wasn't being stripped correctly
14+
// Note: The original content should NOT have line numbers
15+
const originalContent = ` ContactRelaEnum.MS.name(), addressList, contactStatusMap, SensitiveType.ADDRESS, curSector);
16+
}
17+
}
18+
List<ContactInfoItemResp> addressInfoList = new ArrayList<>(CollectionUtils.size(repairInfoList) > 10 ? 10
19+
: CollectionUtils.size(repairInfoList) + CollectionUtils.size(homeAddressInfoList)
20+
+ CollectionUtils.size(idNoAddressInfoList) + CollectionUtils.size(workAddressInfoList)
21+
+ CollectionUtils.size(personIdentityInfoList));`
22+
23+
const diffContent = `<<<<<<< SEARCH
24+
1473 | ContactRelaEnum.MS.name(), addressList, contactStatusMap, SensitiveType.ADDRESS, curSector);
25+
1474 | }
26+
1475 | }
27+
1476 | List<ContactInfoItemResp> addressInfoList = new ArrayList<>(CollectionUtils.size(repairInfoList) > 10 ? 10
28+
1477 | : CollectionUtils.size(repairInfoList) + CollectionUtils.size(homeAddressInfoList)
29+
1478 | + CollectionUtils.size(idNoAddressInfoList) + CollectionUtils.size(workAddressInfoList)
30+
1479 | + CollectionUtils.size(personIdentityInfoList));
31+
=======
32+
1473 | ContactRelaEnum.MS.name(), addressList, contactStatusMap, SensitiveType.ADDRESS, curSector);
33+
1474 | }
34+
1475 | }
35+
1476 |
36+
1477 | //
37+
1478 | if (isAddressDisplayOptimizeEnabled()) {
38+
1479 | homeAddressInfoList = filterAddressesByThreeYearRule(homeAddressInfoList);
39+
1480 | personIdentityInfoList = filterAddressesByThreeYearRule(personIdentityInfoList);
40+
1481 | idNoAddressInfoList = filterAddressesByThreeYearRule(idNoAddressInfoList);
41+
1482 | workAddressInfoList = filterAddressesByThreeYearRule(workAddressInfoList);
42+
1483 | }
43+
1484 |
44+
1485 | List<ContactInfoItemResp> addressInfoList = new ArrayList<>(CollectionUtils.size(repairInfoList) > 10 ? 10
45+
1486 | : CollectionUtils.size(repairInfoList) + CollectionUtils.size(homeAddressInfoList)
46+
1487 | + CollectionUtils.size(idNoAddressInfoList) + CollectionUtils.size(workAddressInfoList)
47+
1488 | + CollectionUtils.size(personIdentityInfoList));
48+
>>>>>>> REPLACE`
49+
50+
const result = await strategy.applyDiff(originalContent, diffContent)
51+
if (!result.success) {
52+
console.error("Test failed with error:", result.error || result.failParts)
53+
}
54+
expect(result.success).toBe(true)
55+
if (result.success) {
56+
const expectedContent = ` ContactRelaEnum.MS.name(), addressList, contactStatusMap, SensitiveType.ADDRESS, curSector);
57+
}
58+
}
59+
60+
//
61+
if (isAddressDisplayOptimizeEnabled()) {
62+
homeAddressInfoList = filterAddressesByThreeYearRule(homeAddressInfoList);
63+
personIdentityInfoList = filterAddressesByThreeYearRule(personIdentityInfoList);
64+
idNoAddressInfoList = filterAddressesByThreeYearRule(idNoAddressInfoList);
65+
workAddressInfoList = filterAddressesByThreeYearRule(workAddressInfoList);
66+
}
67+
68+
List<ContactInfoItemResp> addressInfoList = new ArrayList<>(CollectionUtils.size(repairInfoList) > 10 ? 10
69+
: CollectionUtils.size(repairInfoList) + CollectionUtils.size(homeAddressInfoList)
70+
+ CollectionUtils.size(idNoAddressInfoList) + CollectionUtils.size(workAddressInfoList)
71+
+ CollectionUtils.size(personIdentityInfoList));`
72+
expect(result.content).toBe(expectedContent)
73+
}
74+
})
75+
76+
it("should handle Windows CRLF line endings with line numbers at the last line", async () => {
77+
// Test with Windows-style CRLF line endings
78+
const originalContent = "line 1\r\nline 2\r\nline 3"
79+
const diffContent = `<<<<<<< SEARCH
80+
1 | line 1
81+
2 | line 2
82+
3 | line 3
83+
=======
84+
1 | line 1
85+
2 | modified line 2
86+
3 | line 3
87+
>>>>>>> REPLACE`
88+
89+
const result = await strategy.applyDiff(originalContent, diffContent)
90+
expect(result.success).toBe(true)
91+
if (result.success) {
92+
expect(result.content).toBe("line 1\r\nmodified line 2\r\nline 3")
93+
}
94+
})
95+
96+
it("should handle single line with line number at the end", async () => {
97+
// Simple test case for a single line with line number
98+
const originalContent = "some content here"
99+
const diffContent = `<<<<<<< SEARCH
100+
1 | some content here
101+
=======
102+
1 | modified content here
103+
>>>>>>> REPLACE`
104+
105+
const result = await strategy.applyDiff(originalContent, diffContent)
106+
expect(result.success).toBe(true)
107+
if (result.success) {
108+
expect(result.content).toBe("modified content here")
109+
}
110+
})
111+
})
112+
})

‎src/core/diff/strategies/__tests__/multi-search-replace-simple.spec.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace"
2+
3+
describe("MultiSearchReplaceDiffStrategy - Simple Line Number Test", () => {
4+
let strategy: MultiSearchReplaceDiffStrategy
5+
6+
beforeEach(() => {
7+
strategy = new MultiSearchReplaceDiffStrategy()
8+
})
9+
10+
it("should handle simple line number stripping", async () => {
11+
const originalContent = "line 1\nline 2\nline 3"
12+
const diffContent = `<<<<<<< SEARCH
13+
1 | line 1
14+
2 | line 2
15+
3 | line 3
16+
=======
17+
1 | line 1
18+
2 | modified line 2
19+
3 | line 3
20+
>>>>>>> REPLACE`
21+
22+
const result = await strategy.applyDiff(originalContent, diffContent)
23+
console.log("Result:", result)
24+
expect(result.success).toBe(true)
25+
if (result.success) {
26+
expect(result.content).toBe("line 1\nmodified line 2\nline 3")
27+
}
28+
})
29+
})

‎src/integrations/misc/extract-text.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,14 @@ export function stripLineNumbers(content: string, aggressive: boolean = false):
163163

164164
// Join back with original line endings (carriage return (\r) + line feed (\n) or just line feed (\n))
165165
const lineEnding = content.includes("\r\n") ? "\r\n" : "\n"
166-
return processedLines.join(lineEnding)
166+
const result = processedLines.join(lineEnding)
167+
168+
// Preserve the original trailing newline if it existed
169+
// This is important for diffs where the last line might not have a newline
170+
if (content.endsWith("\n") || content.endsWith("\r\n")) {
171+
return result.endsWith(lineEnding) ? result : result + lineEnding
172+
}
173+
return result
167174
}
168175

169176
/**

‎test-strip-line-numbers.js‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// Test script to debug the stripLineNumbers issue
2+
const { stripLineNumbers, everyLineHasLineNumbers } = require("./src/integrations/misc/extract-text")
3+
4+
// Test case from the issue
5+
const searchContent = `1473 | ContactRelaEnum.MS.name(), addressList, contactStatusMap, SensitiveType.ADDRESS, curSector);
6+
1474 | }
7+
1475 | }
8+
1476 | List<ContactInfoItemResp> addressInfoList = new ArrayList<>(CollectionUtils.size(repairInfoList) > 10 ? 10
9+
1477 | : CollectionUtils.size(repairInfoList) + CollectionUtils.size(homeAddressInfoList)
10+
1478 | + CollectionUtils.size(idNoAddressInfoList) + CollectionUtils.size(workAddressInfoList)
11+
1479 | + CollectionUtils.size(personIdentityInfoList));`
12+
13+
const replaceContent = `1473 | ContactRelaEnum.MS.name(), addressList, contactStatusMap, SensitiveType.ADDRESS, curSector);
14+
1474 | }
15+
1475 | }
16+
1476 |
17+
1477 | //
18+
1478 | if (isAddressDisplayOptimizeEnabled()) {
19+
1479 | homeAddressInfoList = filterAddressesByThreeYearRule(homeAddressInfoList);
20+
1480 | personIdentityInfoList = filterAddressesByThreeYearRule(personIdentityInfoList);
21+
1481 | idNoAddressInfoList = filterAddressesByThreeYearRule(idNoAddressInfoList);
22+
1482 | workAddressInfoList = filterAddressesByThreeYearRule(workAddressInfoList);
23+
1483 | }
24+
1484 |
25+
1485 | List<ContactInfoItemResp> addressInfoList = new ArrayList<>(CollectionUtils.size(repairInfoList) > 10 ? 10
26+
1486 | : CollectionUtils.size(repairInfoList) + CollectionUtils.size(homeAddressInfoList)
27+
1487 | + CollectionUtils.size(idNoAddressInfoList) + CollectionUtils.size(workAddressInfoList)
28+
1488 | + CollectionUtils.size(personIdentityInfoList));`
29+
30+
console.log("Testing everyLineHasLineNumbers:")
31+
console.log("Search content has line numbers:", everyLineHasLineNumbers(searchContent))
32+
console.log("Replace content has line numbers:", everyLineHasLineNumbers(replaceContent))
33+
34+
console.log("\nTesting stripLineNumbers:")
35+
const strippedSearch = stripLineNumbers(searchContent)
36+
const strippedReplace = stripLineNumbers(replaceContent)
37+
38+
console.log("\nOriginal search last line:")
39+
console.log(JSON.stringify(searchContent.split("\n").slice(-1)[0]))
40+
41+
console.log("\nStripped search last line:")
42+
console.log(JSON.stringify(strippedSearch.split("\n").slice(-1)[0]))
43+
44+
console.log("\nOriginal replace last line:")
45+
console.log(JSON.stringify(replaceContent.split("\n").slice(-1)[0]))
46+
47+
console.log("\nStripped replace last line:")
48+
console.log(JSON.stringify(strippedReplace.split("\n").slice(-1)[0]))
49+
50+
// Test with content that doesn't end with newline
51+
const testWithoutNewline =
52+
"1479 | + CollectionUtils.size(personIdentityInfoList));"
53+
console.log("\nTest without trailing newline:")
54+
console.log("Has line numbers:", everyLineHasLineNumbers(testWithoutNewline))
55+
console.log("Stripped:", JSON.stringify(stripLineNumbers(testWithoutNewline)))

0 commit comments

Comments
 (0)