Skip to content

fix(renderer): check if current buffer is loaded #1418

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

Closed
wants to merge 4 commits into from
Closed

fix(renderer): check if current buffer is loaded #1418

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
19e3319
fix(renderer): check if current buffer is loaded
pysan3 Mar 20, 2024
7e1c333
fix(renderer): supply state.bufnr to tree on initialization
pysan3 Mar 20, 2024
934935a
fix(renderer): don't close buffer when toggled on
pysan3 Mar 21, 2024
5568709
fix(renderer): bufnr not closed in time
pysan3 Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions lua/neo-tree/ui/renderer.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,15 @@ M.close = function(state, focus_prior_window)
end
state.winid = nil
end
local bufnr = utils.get_value(state, "bufnr", 0, true)
state.bufnr = nil
vim.schedule(function()
if bufnr > 0 and vim.api.nvim_buf_is_valid(bufnr) then
vim.api.nvim_buf_delete(bufnr, { force = true })
end
end)
if window_existed then
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does it matter if the window existed? Why should a buffer be allowed to remain if the window was closed by some other means, but not if it was closed by this function?

Also, why is it always scheduled even though it usually should be deleted synchronously?

I think the code from 3.22 was already correct and would fix this issue and we should just go back to that:

neo-tree.nvim/lua/neo-tree/ui/renderer.lua

Lines 167 to 175 in 8afbb06

if bufnr > 0 and vim.api.nvim_buf_is_valid(bufnr) then
state.bufnr = nil
local success, err = pcall(vim.api.nvim_buf_delete, bufnr, { force = true })
if not success and err:match("E523") then
vim.schedule_wrap(function()
vim.api.nvim_buf_delete(bufnr, { force = true })
end)()
end
end

Whatever the other edge case was which the changes were trying to fix should be handled another way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have confirmed that the old code works correctly and the bug in #1415 was the direct result of changing that code. I'm going to close this because reverting that code is the real fix.

local bufnr = utils.get_value(state, "bufnr", 0, true)
state.bufnr = nil
vim.schedule(function()
if bufnr > 0 and vim.api.nvim_buf_is_valid(bufnr) then
vim.api.nvim_buf_delete(bufnr, { force = true })
end
end)
end
return window_existed
end

Expand Down Expand Up @@ -778,6 +780,7 @@ create_tree = function(state)
state.tree = NuiTree({
ns_id = highlights.ns_id,
winid = state.winid,
bufnr = state.bufnr,
get_node_id = function(node)
return node.id
end,
Expand Down Expand Up @@ -1010,6 +1013,9 @@ M.acquire_window = function(state)
vim.api.nvim_win_set_buf(state.winid, state.bufnr)
else
close_old_window()
if state.bufnr and vim.api.nvim_buf_is_valid(state.bufnr) then
Copy link
Contributor

Choose a reason for hiding this comment

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

M.close is already deleting the buffer. Why are we double deleting? Is it because of your other change which will only sometimes delete the old buffer? It seems like these two changes are in conflict with another.

Also, if i remember correctly, the original code from a few months ago already did what we are slowly getting back to, which is to immediately attempt to delete the buffer and then schedule a delete for later only if we get a specific error.

I think the right place to make this fix is in the M.close() code, we shouldn't be deleting the buffer multiple times and from different locations.

vim.api.nvim_buf_delete(state.bufnr, { force = true })
end
win = NuiSplit(win_options)
win:mount()
state.bufnr = win.bufnr
Expand Down Expand Up @@ -1087,7 +1093,7 @@ M.window_exists = function(state)
window_exists = false
elseif position == "current" then
window_exists = vim.api.nvim_win_is_valid(winid)
and vim.api.nvim_buf_is_valid(bufnr)
and vim.api.nvim_buf_is_loaded(bufnr)
and vim.api.nvim_win_get_buf(winid) == bufnr
else
local isvalid = M.is_window_valid(winid)
Expand Down
Loading