HiPhish / nvim-ts-rainbow2

Rainbow delimiters for Neovim through Tree-sitter
https://gitlab.com/HiPhish/nvim-ts-rainbow2
Apache License 2.0
339 stars 35 forks source link

Treesitter detach callback: Vim:Error executing lua callback: Vim:E315: ml_get: Invalid lnum: 1 #40

Closed luozhiya closed 1 year ago

luozhiya commented 1 year ago

Hi,

Describe the bug

When using Bdelete to delete a buffer, rainbow will throw an error. In my opinion rainbow needs to register a detach callback.

Thanks.

Error executing Lua callback: ...te/pack/paqs/start/bufdelete.nvim/lua/bufdelete/init.lua:159: Vim:Error executing lua callback: Vim:E315: m
l_get: Invalid lnum: 1
stack traceback:
        [C]: in function 'bufload'
        vim/_editor.lua: in function 'region'
        /usr/local/share/nvim/runtime/lua/vim/highlight.lua:30: in function 'range'
        .../pack/paqs/start/nvim-ts-rainbow2/lua/ts-rainbow/lib.lua:92: in function 'highlight'
        ...tart/nvim-ts-rainbow2/lua/ts-rainbow/strategy/global.lua:29: in function 'highlight_matches'
        ...tart/nvim-ts-rainbow2/lua/ts-rainbow/strategy/global.lua:79: in function 'update_range'
        ...tart/nvim-ts-rainbow2/lua/ts-rainbow/strategy/global.lua:104: in function 'cb'
        ...l/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:736: in function '_do_callback'
        ...l/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:176: in function 'invalidate'
        ...l/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:877: in function '_on_detach'
        /usr/local/share/nvim/runtime/lua/vim/treesitter.lua:73: in function </usr/local/share/nvim/runtime/lua/vim/treesitter.lua:69>

Neovim Version

NVIM v0.10.0-dev-282+gca5a810c4

Steps to reproduce

minimal

local fn = vim.fn
local install_path = fn.stdpath('data') .. '/site/pack/paqs/start/paq-nvim'

if fn.empty(fn.glob(install_path)) > 0 then
  fn.system({ 'git', 'clone', '--depth=1', 'https://github.com/savq/paq-nvim.git', install_path })
end

require('paq')({
  'savq/paq-nvim',
  'nvim-treesitter/nvim-treesitter',
  'HiPhish/nvim-ts-rainbow2',
  'famiu/bufdelete.nvim',
})

require('nvim-treesitter.configs').setup {
  ensure_installed = {
    'lua',
  },
  rainbow = {
    enable = true,
  }
}

Expected behavior

delete buffer without throw an error.

References

share/nvim/runtime/lua/vim/treesitter/languagetree.lua:877

---@package
function LanguageTree:_on_detach(...)
  self:invalidate(true)
  self:_do_callback('detach', ...)
end

https://github.com/neovim/neovim/issues/21416

HiPhish commented 1 year ago

The on_detach callback is not documented (:h LanguageTree:register_cbs()). You do raise a good point though: if a buffer is deleted the current callbacks are never called, which is a resource leak. From reading the source it seems that on_detach is too radical, it will fire even if the user executes :edit. Perhaps a buffer_is_loaded() check at the beginning will do the trick.

HiPhish commented 1 year ago

@luozhiya Can you please try the on-detach branch and report back if the problem still happens?

akinsho commented 1 year ago

@HiPhish I've been experiencing this issue as well and tried your branch but it still happens on that branch. It does seem to be fixed on @luozhiya's branch though i.e. #42

HiPhish commented 1 year ago

@akinsho That branch does not really fix the problem, it just shoves it under the rug. There seems to be a resource leak which :Bdelete is tripping up on and I want to plug the leak. My last commit did plug a leak, but apparently it was not enough. I will have to install famiu/bufdelete.nvim myself and see what the command does.

HiPhish commented 1 year ago

I have been able to narrow down the issue. It only happens in the current dev builds, not in 0.9. If I have two windows with a different buffer for each and I execute :bdelete on one of them I get the error.

HiPhish commented 1 year ago

I have pinned down the problem: In Neovim 0.10 deleting a buffer emits an on_changedtree event (see :h LanguageTree:register_cbs()), but since the buffer is deleted this raises all sort of problems. So the issue is not a resource leak after all. There are two possibilities:

1) This is a bug in Neovim 2) This is a deliberate choice in Neovim

If it is the former I can sit back and wait. If it is the latter I will have to handle this particular edge case like in the PR posted by the OP. I will file an issue in Neovim's repo, then we will see.

HiPhish commented 1 year ago

The PR has been merged. Please test if the problem has been solved on master and close this issue if everything is correct now.