Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(treesitter): clip end row early #28460

Merged
merged 2 commits into from
May 7, 2024
Merged

fix(treesitter): clip end row early #28460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1590e21
refactor(fold): avoid coverity false-positive
tomtomjhj Apr 22, 2024
823b639
fix(treesitter): clip end row early
tomtomjhj Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 9 additions & 16 deletions runtime/lua/vim/treesitter/_fold.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,20 +103,6 @@ local function edit_range(range, srow, erow_old, erow_new)
range[2] = math.max(range[2], erow_new)
end

--- If a parser doesn't have any ranges explicitly set, treesitter will
--- return a range with end_row and end_bytes with a value of UINT32_MAX,
--- so clip end_row to the max buffer line.
---
--- TODO(lewis6991): Handle this generally
---
--- @param bufnr integer
--- @param erow integer? 0-indexed, exclusive
--- @return integer
local function normalise_erow(bufnr, erow)
local max_erow = api.nvim_buf_line_count(bufnr)
return math.min(erow or max_erow, max_erow)
end

-- TODO(lewis6991): Setup a decor provider so injections folds can be parsed
-- as the window is redrawn
---@param bufnr integer
Expand All @@ -126,7 +112,7 @@ end
---@param parse_injections? boolean
local function compute_folds_levels(bufnr, info, srow, erow, parse_injections)
srow = srow or 0
erow = normalise_erow(bufnr, erow)
erow = erow or api.nvim_buf_line_count(bufnr)

local parser = ts.get_parser(bufnr)

Expand Down Expand Up @@ -314,9 +300,16 @@ end
local function on_changedtree(bufnr, foldinfo, tree_changes)
schedule_if_loaded(bufnr, function()
local srow_upd, erow_upd ---@type integer?, integer?
local max_erow = api.nvim_buf_line_count(bufnr)
for _, change in ipairs(tree_changes) do
local srow, _, erow, ecol = Range.unpack4(change)
if ecol > 0 then
-- If a parser doesn't have any ranges explicitly set, treesitter will
-- return a range with end_row and end_bytes with a value of UINT32_MAX,
-- so clip end_row to the max buffer line.
-- TODO(lewis6991): Handle this generally
if erow > max_erow then
erow = max_erow
elseif ecol > 0 then
erow = erow + 1
end
-- Start from `srow - foldminlines`, because this edit may have shrunken the fold below limit.
Expand Down
24 changes: 13 additions & 11 deletions src/nvim/lua/stdlib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,19 +549,21 @@ static int nlua_iconv(lua_State *lstate)
static int nlua_foldupdate(lua_State *lstate)
{
handle_T window = (handle_T)luaL_checkinteger(lstate, 1);
Error err = ERROR_INIT;
win_T *win = find_window_by_handle(window, &err);
if (ERROR_SET(&err)) {
nlua_push_errstr(lstate, err.msg);
api_clear_error(&err);
lua_error(lstate);
return 0;
win_T *win = handle_get_window(window);
if (!win) {
return luaL_error(lstate, "invalid window");
Comment on lines +552 to +554
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's a false positive (i.e. "can never happen"), assert() or abort() is fine too

Copy link
Sponsor Contributor Author

@tomtomjhj tomtomjhj Apr 22, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_window_by_handle in the old version uses handle_get_window and does null check internally, and sets err in that case. So ERROR_SET(&err) in the old version implies win is non-null. But adding assert to that is somewhat redundant, so I replaced those stuff with handle_get_window and direct null check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's redundant either way, because that warning was a false positive.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but I just wanted to reduce the noise from the warning

}
// input is zero-based end-exclusive range
linenr_T top = (linenr_T)luaL_checkinteger(lstate, 2) + 1;
if (top < 1 || top > win->w_buffer->b_ml.ml_line_count) {
return luaL_error(lstate, "invalid top");
}
linenr_T bot = (linenr_T)luaL_checkinteger(lstate, 3);
if (top > bot) {
return luaL_error(lstate, "invalid bot");
}

linenr_T start = (linenr_T)luaL_checkinteger(lstate, 2);
linenr_T end = (linenr_T)luaL_checkinteger(lstate, 3);

foldUpdate(win, start + 1, end);
foldUpdate(win, top, bot);

return 0;
}
Expand Down