Skip to content

Commit acd1575

Browse files
committed
refactor(renderer): extract extmark_clear_range and avoid redundant clears
Extract extmark_clear_range to compute the range of extmarks to clear and return clear_start/clear_end. Change apply_extmarks to accept a skip_clear flag so callers can control when clearing happens. Clear extmarks once before writing lines in upsert_message_now and upsert_part_now, and during bulk flush. Add unit tests to assert extmarks are cleared before replay and that clear_extmarks is called before set_lines.
1 parent 7306c13 commit acd1575

4 files changed

Lines changed: 115 additions & 8 deletions

File tree

lua/opencode/ui/renderer/buffer.lua

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ local function highlight_written_lines(start_line, lines)
129129
output_window.highlight_changed_lines(start_line, start_line + #lines - 1)
130130
end
131131

132-
local function apply_extmarks(previous_formatted, formatted_data, line_start, old_line_end, new_line_end)
132+
local function extmark_clear_range(previous_formatted, formatted_data, line_start, old_line_end, new_line_end)
133133
local prefix_len = math.min(
134134
unchanged_prefix_len(previous_formatted, formatted_data),
135135
unchanged_extmark_prefix_len(previous_formatted, formatted_data)
@@ -162,13 +162,21 @@ local function apply_extmarks(previous_formatted, formatted_data, line_start, ol
162162
if next_min_extmark ~= nil then
163163
clear_start = math.min(clear_start, line_start + next_min_extmark)
164164
end
165+
165166
clear_start = math.max(0, clear_start)
166167
local clear_end = math.max(
167168
max_extmark_line(previous_formatted, old_line_end),
168169
max_extmark_line(formatted_data, new_line_end)
169170
) + 1
170171

171-
output_window.clear_extmarks(clear_start, clear_end)
172+
return clear_start, clear_end
173+
end
174+
175+
local function apply_extmarks(previous_formatted, formatted_data, line_start, old_line_end, new_line_end, skip_clear)
176+
local clear_start, clear_end = extmark_clear_range(previous_formatted, formatted_data, line_start, old_line_end, new_line_end)
177+
if not skip_clear then
178+
output_window.clear_extmarks(clear_start, clear_end)
179+
end
172180

173181
local extmark_start_line = math.max(0, clear_start - line_start)
174182
local extmarks = slice_extmarks(formatted_data.extmarks, extmark_start_line)
@@ -341,12 +349,20 @@ function M.upsert_message_now(message_id, formatted_data, previous_formatted)
341349
local prefix_len = unchanged_prefix_len(previous_formatted, formatted_data)
342350
local write_start = cached.line_start + prefix_len
343351
local lines_to_write = slice_lines(formatted_data.lines, prefix_len + 1)
344-
352+
local clear_start, clear_end = extmark_clear_range(
353+
previous_formatted,
354+
formatted_data,
355+
cached.line_start,
356+
old_line_end,
357+
cached.line_start + #formatted_data.lines - 1
358+
)
359+
360+
output_window.clear_extmarks(clear_start, clear_end)
345361
output_window.set_lines(lines_to_write, write_start, cached.line_end + 1)
346362
highlight_written_lines(write_start, lines_to_write)
347363

348364
local new_line_end = cached.line_start + #formatted_data.lines - 1
349-
apply_extmarks(previous_formatted, formatted_data, cached.line_start, old_line_end, new_line_end)
365+
apply_extmarks(previous_formatted, formatted_data, cached.line_start, old_line_end, new_line_end, true)
350366
ctx.render_state:set_message(cached.message, cached.line_start, new_line_end)
351367

352368
local delta = new_line_end - old_line_end
@@ -399,7 +415,15 @@ function M.upsert_part_now(part_id, message_id, formatted_data, previous_formatt
399415
local prefix_len = unchanged_prefix_len(previous_formatted, formatted_data)
400416
local write_start = cached.line_start + prefix_len
401417
local lines_to_write = slice_lines(formatted_data.lines, prefix_len + 1)
402-
418+
local clear_start, clear_end = extmark_clear_range(
419+
previous_formatted,
420+
formatted_data,
421+
cached.line_start,
422+
old_line_end,
423+
cached.line_start + #formatted_data.lines - 1
424+
)
425+
426+
output_window.clear_extmarks(clear_start, clear_end)
403427
output_window.set_lines(lines_to_write, write_start, cached.line_end + 1)
404428
highlight_written_lines(write_start, lines_to_write)
405429

@@ -409,7 +433,7 @@ function M.upsert_part_now(part_id, message_id, formatted_data, previous_formatt
409433
if new_line_end ~= cached.line_end then
410434
ctx.render_state:update_part_lines(part_id, cached.line_start, new_line_end)
411435
end
412-
apply_extmarks(previous_formatted, formatted_data, cached.line_start, old_line_end, new_line_end)
436+
apply_extmarks(previous_formatted, formatted_data, cached.line_start, old_line_end, new_line_end, true)
413437
set_part_extmark_state(part_id, formatted_data)
414438
return true
415439
end

lua/opencode/ui/renderer/flush.lua

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,8 @@ function M.end_bulk_mode()
436436
output_window.set_lines(lines, 0, -1)
437437
end)
438438

439+
output_window.clear_extmarks()
440+
439441
if next(ctx.bulk_extmarks_by_line) then
440442
output_window.set_extmarks(ctx.bulk_extmarks_by_line, 0)
441443
end

tests/unit/output_window_spec.lua

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,37 @@ describe('renderer flush cleanup', function()
251251
assert.stub(set_extmarks_stub).was_not_called()
252252
end)
253253
end)
254+
255+
describe('renderer bulk flush extmarks', function()
256+
local buf
257+
258+
before_each(function()
259+
buf = vim.api.nvim_create_buf(false, true)
260+
state.ui.set_windows({ output_buf = buf })
261+
vim.api.nvim_buf_set_lines(buf, 0, -1, false, { 'old header', 'old body', '' })
262+
vim.api.nvim_buf_set_extmark(buf, output_window.namespace, 2, 0, {
263+
virt_text = { { 'OLD', 'Normal' } },
264+
virt_text_pos = 'overlay',
265+
})
266+
end)
267+
268+
after_each(function()
269+
local ctx = require('opencode.ui.renderer.ctx')
270+
ctx:reset()
271+
state.ui.set_windows(nil)
272+
pcall(vim.api.nvim_buf_delete, buf, { force = true })
273+
end)
274+
275+
it('clears stale extmarks before replaying bulk extmarks', function()
276+
local ctx = require('opencode.ui.renderer.ctx')
277+
278+
flush.begin_bulk_mode()
279+
ctx.bulk_buffer_lines = { 'new header' }
280+
281+
flush.end_bulk_mode()
282+
283+
local marks = vim.api.nvim_buf_get_extmarks(buf, output_window.namespace, 0, -1, { details = true })
284+
assert.equals(0, #marks)
285+
assert.same({ 'new header', '' }, vim.api.nvim_buf_get_lines(buf, 0, -1, false))
286+
end)
287+
end)

tests/unit/renderer_buffer_spec.lua

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,40 @@ local ctx = require('opencode.ui.renderer.ctx')
33
local output_window = require('opencode.ui.output_window')
44
local stub = require('luassert.stub')
55

6+
local function assert_called_before(call_order, first_name, second_name)
7+
local first_idx
8+
local second_idx
9+
10+
for idx, name in ipairs(call_order) do
11+
if name == first_name and not first_idx then
12+
first_idx = idx
13+
end
14+
if name == second_name and not second_idx then
15+
second_idx = idx
16+
end
17+
end
18+
19+
assert.is_truthy(first_idx, 'expected ' .. first_name .. ' to be called')
20+
assert.is_truthy(second_idx, 'expected ' .. second_name .. ' to be called')
21+
assert.is_true(first_idx < second_idx)
22+
end
23+
624
describe('renderer.buffer extmarks', function()
725
local set_lines_stub
826
local clear_extmarks_stub
927
local set_extmarks_stub
1028
local highlight_changed_lines_stub
29+
local call_order
1130

1231
before_each(function()
1332
ctx:reset()
14-
set_lines_stub = stub(output_window, 'set_lines')
15-
clear_extmarks_stub = stub(output_window, 'clear_extmarks')
33+
call_order = {}
34+
set_lines_stub = stub(output_window, 'set_lines').invokes(function()
35+
call_order[#call_order + 1] = 'set_lines'
36+
end)
37+
clear_extmarks_stub = stub(output_window, 'clear_extmarks').invokes(function()
38+
call_order[#call_order + 1] = 'clear_extmarks'
39+
end)
1640
set_extmarks_stub = stub(output_window, 'set_extmarks')
1741
highlight_changed_lines_stub = stub(output_window, 'highlight_changed_lines')
1842
end)
@@ -48,6 +72,7 @@ describe('renderer.buffer extmarks', function()
4872
{ line_hl_group = 'OpencodeReasoningText' },
4973
},
5074
}, 11)
75+
assert_called_before(call_order, 'clear_extmarks', 'set_lines')
5176
end)
5277

5378
it('reapplies extmarks at the correct line after unchanged leading lines', function()
@@ -77,5 +102,27 @@ describe('renderer.buffer extmarks', function()
77102
{ line_hl_group = 'OpencodeDialogOptionHover' },
78103
},
79104
}, 24)
105+
assert_called_before(call_order, 'clear_extmarks', 'set_lines')
106+
end)
107+
108+
it('clears extmarks before rewriting a message', function()
109+
ctx.render_state:set_message({ info = { id = 'msg_1' } }, 30, 31)
110+
111+
buffer.upsert_message_now('msg_1', {
112+
lines = { 'alpha', '' },
113+
extmarks = {},
114+
actions = {},
115+
}, {
116+
lines = { 'alpha', 'beta' },
117+
extmarks = {
118+
[1] = {
119+
{ virt_text = { { 'OLD', 'Normal' } }, virt_text_pos = 'overlay' },
120+
},
121+
},
122+
actions = {},
123+
})
124+
125+
assert.stub(clear_extmarks_stub).was_called()
126+
assert_called_before(call_order, 'clear_extmarks', 'set_lines')
80127
end)
81128
end)

0 commit comments

Comments
 (0)