GitHub/neovim
1
0
Fork 0
mirror of https://github.com/neovim/neovim synced 2025-07-15 16:51:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(api): wrong return value with reverse range + overlap #32956

Browse Source 
Problem:  When iterating in reverse with {start} > {end} in
          `nvim_buf_get_extmarks()`, marks that overlap {start} and are
          greater than {end} are included in the return value twice.
          Marks that overlap {end} and do not overlap {start} are not
          not included in the return value at all. Marks are not
          actually returned in a meaningful "traversal order".

Solution: Rather than actually iterating in reverse, (also possible but
          requires convoluted conditions and would require fetching
          overlapping marks for both the {start} and {end} position,
          while still ending up with non-traversal ordered marks),
          iterate normally and reverse the return value.
(cherry picked from commit 65170e8dad)
This commit is contained in:
luukvbaal
2025-04-22 01:18:03 +02:00
committed by github-actions[bot]
parent 3b0c88a537
commit fb71d631a5
5 changed files with 62 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
runtime/doc/api.txt
View File

@ -2777,8 +2777,11 @@ nvim_buf_get_extmarks({buffer}, {ns_id}, {start}, {end}, {opts})
vim.api.nvim_buf_get_extmarks(0, my_ns, {0,0}, {-1,-1}, {})
<
If `end` is less than `start`, traversal works backwards. (Useful with
`limit`, to get the first marks prior to a given position.)
If `end` is less than `start`, marks are returned in reverse order.
(Useful with `limit`, to get the first marks prior to a given position.)
Note: For a reverse range, `limit` does not actually affect the traversed
range, just how many marks are returned
Note: when using extmark ranges (marks with a end_row/end_col position)
the `overlap` option might be useful. Otherwise only the start position of

7
runtime/lua/vim/_meta/api.lua generated
View File

@ -369,8 +369,11 @@ function vim.api.nvim_buf_get_extmark_by_id(buffer, ns_id, id, opts) end
--- vim.api.nvim_buf_get_extmarks(0, my_ns, {0,0}, {-1,-1}, {})
--- ```
---
--- If `end` is less than `start`, traversal works backwards. (Useful
--- with `limit`, to get the first marks prior to a given position.)
--- If `end` is less than `start`, marks are returned in reverse order.
--- (Useful with `limit`, to get the first marks prior to a given position.)
---
--- Note: For a reverse range, `limit` does not actually affect the traversed
--- range, just how many marks are returned
---
--- Note: when using extmark ranges (marks with a end_row/end_col position)
--- the `overlap` option might be useful. Otherwise only the start position

35
src/nvim/api/extmark.c
View File

@ -241,8 +241,11 @@ ArrayOf(Integer) nvim_buf_get_extmark_by_id(Buffer buffer, Integer ns_id,
/// vim.api.nvim_buf_get_extmarks(0, my_ns, {0,0}, {-1,-1}, {})
/// ```
///
/// If `end` is less than `start`, traversal works backwards. (Useful
/// with `limit`, to get the first marks prior to a given position.)
/// If `end` is less than `start`, marks are returned in reverse order.
/// (Useful with `limit`, to get the first marks prior to a given position.)
///
/// Note: For a reverse range, `limit` does not actually affect the traversed
/// range, just how many marks are returned
///
/// Note: when using extmark ranges (marks with a end_row/end_col position)
/// the `overlap` option might be useful. Otherwise only the start position
@ -327,8 +330,6 @@ Array nvim_buf_get_extmarks(Buffer buffer, Integer ns_id, Object start, Object e
limit = INT64_MAX;
}
bool reverse = false;
int l_row;
colnr_T l_col;
if (!extmark_get_index_from_obj(buf, ns_id, start, &l_row, &l_col, err)) {
@ -341,17 +342,31 @@ Array nvim_buf_get_extmarks(Buffer buffer, Integer ns_id, Object start, Object e
return rv;
}
if (l_row > u_row || (l_row == u_row && l_col > u_col)) {
reverse = true;
size_t rv_limit = (size_t)limit;
bool reverse = l_row > u_row || (l_row == u_row && l_col > u_col);
if (reverse) {
limit = INT64_MAX; // limit the return value instead
int row = l_row;
l_row = u_row;
u_row = row;
colnr_T col = l_col;
l_col = u_col;
u_col = col;
}
// note: ns_id=-1 allowed, represented as UINT32_MAX
ExtmarkInfoArray marks = extmark_get(buf, (uint32_t)ns_id, l_row, l_col, u_row,
u_col, (int64_t)limit, reverse, type, opts->overlap);
u_col, (int64_t)limit, type, opts->overlap);
rv = arena_array(arena, kv_size(marks));
for (size_t i = 0; i < kv_size(marks); i++) {
ADD_C(rv, ARRAY_OBJ(extmark_to_array(kv_A(marks, i), true, details, hl_name, arena)));
rv = arena_array(arena, MIN(kv_size(marks), rv_limit));
if (reverse) {
for (int i = (int)kv_size(marks) - 1; i >= 0 && kv_size(rv) < rv_limit; i--) {
ADD_C(rv, ARRAY_OBJ(extmark_to_array(kv_A(marks, i), true, details, hl_name, arena)));
}
} else {
for (size_t i = 0; i < kv_size(marks); i++) {
ADD_C(rv, ARRAY_OBJ(extmark_to_array(kv_A(marks, i), true, details, hl_name, arena)));
}
}
kv_destroy(marks);

26
src/nvim/extmark.c
View File

@ -259,11 +259,9 @@ bool extmark_clear(buf_T *buf, uint32_t ns_id, int l_row, colnr_T l_col, int u_r
///
/// if upper_lnum or upper_col are negative the buffer
/// will be searched to the start, or end
/// reverse can be set to control the order of the array
/// amount = amount of marks to find or INT64_MAX for all
ExtmarkInfoArray extmark_get(buf_T *buf, uint32_t ns_id, int l_row, colnr_T l_col, int u_row,
colnr_T u_col, int64_t amount, bool reverse, ExtmarkType type_filter,
bool overlap)
colnr_T u_col, int64_t amount, ExtmarkType type_filter, bool overlap)
{
ExtmarkInfoArray array = KV_INITIAL_VALUE;
MarkTreeIter itr[1];
@ -281,29 +279,21 @@ ExtmarkInfoArray extmark_get(buf_T *buf, uint32_t ns_id, int l_row, colnr_T l_co
} else {
// Find all the marks beginning with the start position
marktree_itr_get_ext(buf->b_marktree, MTPos(l_row, l_col),
itr, reverse, false, NULL, NULL);
itr, false, false, NULL, NULL);
}
int order = reverse ? -1 : 1;
while ((int64_t)kv_size(array) < amount) {
MTKey mark = marktree_itr_current(itr);
if (mark.pos.row < 0
|| (mark.pos.row - u_row) * order > 0
|| (mark.pos.row == u_row && (mark.pos.col - u_col) * order > 0)) {
|| (mark.pos.row > u_row)
|| (mark.pos.row == u_row && mark.pos.col > u_col)) {
break;
}
if (mt_end(mark)) {
goto next_mark;
}
MTKey end = marktree_get_alt(buf->b_marktree, mark, NULL);
push_mark(&array, ns_id, type_filter, mtpair_from(mark, end));
next_mark:
if (reverse) {
marktree_itr_prev(buf->b_marktree, itr);
} else {
marktree_itr_next(buf->b_marktree, itr);
if (!mt_end(mark)) {
MTKey end = marktree_get_alt(buf->b_marktree, mark, NULL);
push_mark(&array, ns_id, type_filter, mtpair_from(mark, end));
}
marktree_itr_next(buf->b_marktree, itr);
}
return array;
}

30
test/functional/api/extmark_spec.lua
View File

@ -331,30 +331,30 @@ describe('API/extmarks', function()
rv = get_extmarks(ns, lower, upper)
eq({ { marks[3], positions[3][1], positions[3][2] } }, rv)
-- prev with mark id
-- reverse with mark id
rv = get_extmarks(ns, marks[3], { 0, 0 }, { limit = 1 })
eq({ { marks[3], positions[3][1], positions[3][2] } }, rv)
rv = get_extmarks(ns, marks[2], { 0, 0 }, { limit = 1 })
eq({ { marks[2], positions[2][1], positions[2][2] } }, rv)
-- prev with positional when mark exists at position
-- reverse with positional when mark exists at position
rv = get_extmarks(ns, positions[3], { 0, 0 }, { limit = 1 })
eq({ { marks[3], positions[3][1], positions[3][2] } }, rv)
-- prev with positional index (no mark at position)
-- reverse with positional index (no mark at position)
rv = get_extmarks(ns, { positions[1][1], positions[1][2] + 1 }, { 0, 0 }, { limit = 1 })
eq({ { marks[1], positions[1][1], positions[1][2] } }, rv)
-- prev with Extremity index
-- reverse with Extremity index
rv = get_extmarks(ns, { -1, -1 }, { 0, 0 }, { limit = 1 })
eq({ { marks[3], positions[3][1], positions[3][2] } }, rv)
-- prevrange with mark id
-- reverse with mark id
rv = get_extmarks(ns, marks[3], marks[1])
eq({ marks[3], positions[3][1], positions[3][2] }, rv[1])
eq({ marks[2], positions[2][1], positions[2][2] }, rv[2])
eq({ marks[1], positions[1][1], positions[1][2] }, rv[3])
-- prevrange with limit
-- reverse with limit
rv = get_extmarks(ns, marks[3], marks[1], { limit = 2 })
eq(2, #rv)
-- prevrange with positional when mark exists at position
-- reverse with positional when mark exists at position
rv = get_extmarks(ns, positions[3], positions[1])
eq({
{ marks[3], positions[3][1], positions[3][2] },
@ -363,7 +363,7 @@ describe('API/extmarks', function()
}, rv)
rv = get_extmarks(ns, positions[2], positions[1])
eq(2, #rv)
-- prevrange with positional index (no mark at position)
-- reverse with positional index (no mark at position)
lower = { positions[2][1], positions[2][2] + 1 }
upper = { positions[3][1], positions[3][2] + 1 }
rv = get_extmarks(ns, upper, lower)
@ -372,7 +372,7 @@ describe('API/extmarks', function()
upper = { positions[3][1], positions[3][2] + 2 }
rv = get_extmarks(ns, upper, lower)
eq({}, rv)
-- prevrange with extremity index
-- reverse with extremity index
lower = { 0, 0 }
upper = { positions[2][1], positions[2][2] - 1 }
rv = get_extmarks(ns, upper, lower)
@ -428,6 +428,14 @@ describe('API/extmarks', function()
set_extmark(ns, marks[3], 0, 2) -- check is inclusive
local rv = get_extmarks(ns, { 2, 3 }, { 0, 2 })
eq({ { marks[1], 2, 1 }, { marks[2], 1, 4 }, { marks[3], 0, 2 } }, rv)
-- doesn't include paired marks whose start pos > lower bound twice
-- and returns mark overlapping start pos but not end pos
local m1 = set_extmark(ns, nil, 0, 0, { end_row = 1, end_col = 4 })
local m2 = set_extmark(ns, nil, 0, 0, { end_row = 1, end_col = 2 })
local m3 = set_extmark(ns, nil, 1, 0, { end_row = 1, end_col = 4 })
local m4 = set_extmark(ns, nil, 1, 2, { end_row = 1, end_col = 4 })
rv = get_extmarks(ns, { 1, 3 }, { 1, 2 }, { overlap = true })
eq({ { m4, 1, 2 }, { m3, 1, 0 }, { m2, 0, 0 }, { m1, 0, 0 } }, rv)
end)
it('get_marks limit=0 returns nothing', function()
@ -973,7 +981,7 @@ describe('API/extmarks', function()
eq(1, #rv)
rv = get_extmarks(ns2, { 0, 0 }, positions[2], { limit = 1 })
eq(1, #rv)
-- get_prev (limit set)
-- reverse (limit set)
rv = get_extmarks(ns, positions[1], { 0, 0 }, { limit = 1 })
eq(1, #rv)
rv = get_extmarks(ns2, positions[1], { 0, 0 }, { limit = 1 })
@ -984,7 +992,7 @@ describe('API/extmarks', function()
eq(2, #rv)
rv = get_extmarks(ns2, positions[1], positions[2])
eq(2, #rv)
-- get_prev (no limit)
-- reverse (no limit)
rv = get_extmarks(ns, positions[2], positions[1])
eq(2, #rv)
rv = get_extmarks(ns2, positions[2], positions[1])