Skip to content

Commit b933793

Browse files
robertpiosikrobertpiosik
committed
Refactor conflict marker parser to handle whitespace and improve parameter structure
1 parent c6cb6e4 commit b933793

6 files changed

Lines changed: 160 additions & 91 deletions

File tree

‎apps/editor/src/commands/apply-chat-response-command/handlers/conflict-markers-handler.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,10 @@ export const handle_conflict_markers = async (
9797
try {
9898
let new_content = rename_source_content
9999
if (has_markers) {
100-
new_content = apply_conflict_markers_to_content(
101-
rename_source_content,
102-
file.content
103-
)
100+
new_content = apply_conflict_markers_to_content({
101+
original_content: rename_source_content,
102+
markers_content: file.content
103+
})
104104
} else {
105105
new_content = file.content
106106
}
@@ -151,10 +151,10 @@ export const handle_conflict_markers = async (
151151
try {
152152
const document = await vscode.workspace.openTextDocument(safe_path)
153153
const original_content = document.getText()
154-
const current_content = apply_conflict_markers_to_content(
154+
const current_content = apply_conflict_markers_to_content({
155155
original_content,
156-
file.content
157-
)
156+
markers_content: file.content
157+
})
158158

159159
if (current_content !== original_content) {
160160
await vscode.workspace.fs.writeFile(

‎apps/editor/src/commands/apply-chat-response-command/utils/conflict-marker-parser/conflict-marker-parser.spec.ts‎

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,41 +13,66 @@ describe('conflict-marker-parser', () => {
1313
describe('apply_conflict_markers_to_content', () => {
1414
it('applies a basic conflict marker correctly', () => {
1515
const test_case = 'basic-conflict'
16-
const original = load_test_case_file(test_case, 'original.txt')
17-
const markers = load_test_case_file(test_case, 'markers.txt')
18-
const expected = load_test_case_file(test_case, 'expected.txt')
16+
const original_content = load_test_case_file(test_case, 'original.txt')
17+
const markers_content = load_test_case_file(test_case, 'markers.txt')
18+
const expected_content = load_test_case_file(test_case, 'expected.txt')
1919

20-
const result = apply_conflict_markers_to_content(original, markers)
21-
expect(result).toBe(expected)
20+
const result = apply_conflict_markers_to_content({
21+
original_content,
22+
markers_content
23+
})
24+
expect(result).toBe(expected_content)
2225
})
2326

2427
it('applies multiple conflict markers in a single file', () => {
2528
const test_case = 'multiple-conflicts'
26-
const original = load_test_case_file(test_case, 'original.txt')
27-
const markers = load_test_case_file(test_case, 'markers.txt')
28-
const expected = load_test_case_file(test_case, 'expected.txt')
29+
const original_content = load_test_case_file(test_case, 'original.txt')
30+
const markers_content = load_test_case_file(test_case, 'markers.txt')
31+
const expected_content = load_test_case_file(test_case, 'expected.txt')
2932

30-
const result = apply_conflict_markers_to_content(original, markers)
31-
expect(result).toBe(expected)
33+
const result = apply_conflict_markers_to_content({
34+
original_content,
35+
markers_content
36+
})
37+
expect(result).toBe(expected_content)
3238
})
3339

3440
it('handles deletions via empty updated blocks', () => {
3541
const test_case = 'deletion'
36-
const original = load_test_case_file(test_case, 'original.txt')
37-
const markers = load_test_case_file(test_case, 'markers.txt')
38-
const expected = load_test_case_file(test_case, 'expected.txt')
42+
const original_content = load_test_case_file(test_case, 'original.txt')
43+
const markers_content = load_test_case_file(test_case, 'markers.txt')
44+
const expected_content = load_test_case_file(test_case, 'expected.txt')
3945

40-
const result = apply_conflict_markers_to_content(original, markers)
41-
expect(result).toBe(expected)
46+
const result = apply_conflict_markers_to_content({
47+
original_content,
48+
markers_content
49+
})
50+
expect(result).toBe(expected_content)
4251
})
4352

4453
it('throws an error when context cannot be found', () => {
45-
const original = 'Some random text'
46-
const markers = '<<<<<<<\nMissing\n=======\nFound\n>>>>>>>'
54+
const original_content = 'Some random text'
55+
const markers_content = '<<<<<<<\nMissing\n=======\nFound\n>>>>>>>'
4756

4857
expect(() =>
49-
apply_conflict_markers_to_content(original, markers)
58+
apply_conflict_markers_to_content({
59+
original_content,
60+
markers_content
61+
})
5062
).toThrow(/Could not find content to replace/)
5163
})
64+
65+
it('handles whitespace issues in conflict markers', () => {
66+
const test_case = 'whitespace-issues'
67+
const original_content = load_test_case_file(test_case, 'original.txt')
68+
const markers_content = load_test_case_file(test_case, 'markers.txt')
69+
const expected_content = load_test_case_file(test_case, 'expected.txt')
70+
71+
const result = apply_conflict_markers_to_content({
72+
original_content,
73+
markers_content
74+
})
75+
expect(result).toBe(expected_content)
76+
})
5277
})
5378
})

‎apps/editor/src/commands/apply-chat-response-command/utils/conflict-marker-parser/conflict-marker-parser.ts‎

Lines changed: 100 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ export type Segment =
33
| { type: 'conflict'; original_lines: string[]; updated_lines: string[] }
44

55
export const parse_conflict_segments = (content: string): Segment[] => {
6-
const lines = content.split('\n')
6+
const lines = content.replace(/\r\n/g, '\n').split('\n')
77
const segments: Segment[] = []
88
let current_lines: string[] = []
99

@@ -13,7 +13,8 @@ export const parse_conflict_segments = (content: string): Segment[] => {
1313
let updated_buffer: string[] = []
1414

1515
for (const line of lines) {
16-
if (line.trim().startsWith('<<<<<<<')) {
16+
const trimmed = line.trim()
17+
if (trimmed.startsWith('<<<<<<<')) {
1718
if (state == 'NORMAL') {
1819
if (current_lines.length > 0) {
1920
segments.push({ type: 'common', lines: [...current_lines] })
@@ -29,7 +30,7 @@ export const parse_conflict_segments = (content: string): Segment[] => {
2930
continue
3031
}
3132

32-
if (line.trim().startsWith('=======')) {
33+
if (trimmed.startsWith('=======')) {
3334
if (state == 'ORIGINAL') {
3435
state = 'UPDATED'
3536
} else if (state == 'NORMAL') {
@@ -40,21 +41,12 @@ export const parse_conflict_segments = (content: string): Segment[] => {
4041
continue
4142
}
4243

43-
if (line.trim().startsWith('>>>>>>>')) {
44-
if (state == 'UPDATED') {
44+
if (trimmed.startsWith('>>>>>>>')) {
45+
if (state == 'UPDATED' || state == 'ORIGINAL') {
4546
segments.push({
4647
type: 'conflict',
4748
original_lines: original_buffer,
48-
updated_lines: updated_buffer
49-
})
50-
state = 'NORMAL'
51-
original_buffer = []
52-
updated_buffer = []
53-
} else if (state == 'ORIGINAL') {
54-
segments.push({
55-
type: 'conflict',
56-
original_lines: original_buffer,
57-
updated_lines: []
49+
updated_lines: state == 'UPDATED' ? updated_buffer : []
5850
})
5951
state = 'NORMAL'
6052
original_buffer = []
@@ -74,73 +66,115 @@ export const parse_conflict_segments = (content: string): Segment[] => {
7466
}
7567
}
7668

77-
if (state == 'NORMAL' && current_lines.length > 0) {
69+
if (current_lines.length > 0) {
7870
segments.push({ type: 'common', lines: current_lines })
7971
}
8072

8173
return segments
8274
}
8375

84-
export const apply_conflict_markers_to_content = (
85-
original_content: string,
76+
const escape_reg_exp = (string: string) => {
77+
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
78+
}
79+
80+
export const apply_conflict_markers_to_content = (params: {
81+
original_content: string
8682
markers_content: string
87-
): string => {
88-
let current_content = original_content
89-
const segments = parse_conflict_segments(markers_content)
83+
}): string => {
84+
const line_ending = params.original_content.includes('\r\n') ? '\r\n' : '\n'
85+
let current_content = params.original_content
86+
const segments = parse_conflict_segments(params.markers_content)
9087
let cursor = 0
9188

9289
for (let i = 0; i < segments.length; i++) {
9390
const segment = segments[i]
94-
if (segment.type == 'conflict') {
95-
const prev = i > 0 ? segments[i - 1] : undefined
96-
const next = i < segments.length - 1 ? segments[i + 1] : undefined
97-
98-
const context_before = prev && prev.type == 'common' ? prev.lines : []
99-
const context_after = next && next.type == 'common' ? next.lines : []
100-
101-
const search_lines = [
102-
...context_before,
103-
...segment.original_lines,
104-
...context_after
105-
]
106-
const search_text = search_lines.join('\n')
107-
108-
const replace_lines = [
109-
...context_before,
110-
...segment.updated_lines,
111-
...context_after
112-
]
113-
const replace_text = replace_lines.join('\n')
114-
115-
const index = current_content.indexOf(search_text, cursor)
116-
117-
if (index == -1) {
118-
throw new Error(
119-
`Could not find content to replace for conflict marker. Context:\n${search_text.slice(
120-
0,
121-
100
122-
)}...`
123-
)
124-
}
91+
if (segment.type !== 'conflict') continue
92+
93+
const prev = i > 0 ? segments[i - 1] : undefined
94+
const next = i < segments.length - 1 ? segments[i + 1] : undefined
95+
96+
const context_before = prev && prev.type == 'common' ? prev.lines : []
97+
const context_after = next && next.type == 'common' ? next.lines : []
98+
99+
const all_search_lines = [
100+
...context_before,
101+
...segment.original_lines,
102+
...context_after
103+
]
104+
105+
const all_search_patterns = all_search_lines.map((line) => {
106+
const trimmed = line.trim()
107+
if (trimmed == '') return '[ \\t]*'
108+
const escaped = escape_reg_exp(trimmed)
109+
return `[ \\t]*${escaped}[ \\t]*`
110+
})
111+
112+
const start_original = context_before.length
113+
const end_original = start_original + segment.original_lines.length
114+
115+
const regex_parts: string[] = []
116+
if (context_before.length > 0) {
117+
regex_parts.push(
118+
`(${all_search_patterns.slice(0, start_original).join('\\r?\\n')})`
119+
)
120+
}
121+
if (segment.original_lines.length > 0) {
122+
regex_parts.push(
123+
`(${all_search_patterns
124+
.slice(start_original, end_original)
125+
.join('\\r?\\n')})`
126+
)
127+
}
128+
if (context_after.length > 0) {
129+
regex_parts.push(
130+
`(${all_search_patterns.slice(end_original).join('\\r?\\n')})`
131+
)
132+
}
125133

126-
current_content =
127-
current_content.slice(0, index) +
128-
replace_text +
129-
current_content.slice(index + search_text.length)
134+
const regex = new RegExp(regex_parts.join('\\r?\\n'))
135+
const content_slice = current_content.slice(cursor)
136+
const match = regex.exec(content_slice)
130137

131-
const unique_replaced_lines = [
132-
...context_before,
133-
...segment.updated_lines
134-
]
135-
const unique_replaced_text = unique_replaced_lines.join('\n')
138+
if (!match) {
139+
const search_text = all_search_lines.join('\n')
140+
throw new Error(
141+
`Could not find content to replace for conflict marker. Context:\n${search_text.slice(0, 100)}...`
142+
)
143+
}
136144

137-
let advance = unique_replaced_text.length
138-
if (context_after.length > 0 && unique_replaced_lines.length > 0) {
139-
advance += 1
140-
}
145+
const match_index = cursor + match.index
146+
const matched_text = match[0]
141147

142-
cursor = index + advance
148+
let group_index = 1
149+
const matched_before = context_before.length > 0 ? match[group_index++] : ''
150+
if (segment.original_lines.length > 0) group_index++
151+
const matched_after = context_after.length > 0 ? match[group_index++] : ''
152+
153+
const updated_text = segment.updated_lines.join(line_ending)
154+
155+
const replacement_parts: string[] = []
156+
if (matched_before) replacement_parts.push(matched_before)
157+
if (segment.updated_lines.length > 0) {
158+
replacement_parts.push(updated_text)
143159
}
160+
if (matched_after) replacement_parts.push(matched_after)
161+
162+
const replacement_text = replacement_parts.join(line_ending)
163+
164+
current_content =
165+
current_content.slice(0, match_index) +
166+
replacement_text +
167+
current_content.slice(match_index + matched_text.length)
168+
169+
// Advance cursor while preserving context for the next possible conflict
170+
let advance_length = replacement_text.length
171+
if (context_after.length > 0 && matched_after) {
172+
const offset = replacement_text.lastIndexOf(matched_after)
173+
advance_length = offset > 0 ? offset - line_ending.length : 0
174+
}
175+
176+
cursor = Math.max(0, match_index + advance_length)
144177
}
178+
145179
return current_content
146180
}

‎apps/editor/src/commands/apply-chat-response-command/utils/conflict-marker-parser/test-cases/whitespace-issues/expected.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
context
2+
changed

‎apps/editor/src/commands/apply-chat-response-command/utils/conflict-marker-parser/test-cases/whitespace-issues/markers.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
context
2+
<<<<<<<
3+
change me
4+
=======
5+
changed
6+
>>>>>>>

‎apps/editor/src/commands/apply-chat-response-command/utils/conflict-marker-parser/test-cases/whitespace-issues/original.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
context
2+
change me

0 commit comments

Comments
 (0)