stevearc / stickybuf.nvim

Neovim plugin for locking a buffer to a window
MIT License
246 stars 7 forks source link

Nvim tree broken #13

Closed fengwk closed 1 year ago

fengwk commented 1 year ago

My nvim tree has been experiencing frequent issues and has returned to normal after troubleshooting stickybuf. Below is a screenshot of the nvim tree reporting an error:

image image

I can confirm that this is an issue introduced by 47404e1c5dc6881385652f94672af8ef5645c306 and resolved the nvim tree exception after rolling back to fc75dc22d12e5446c72a0d5f067cd7a16b3d921a.

stevearc commented 1 year ago

@emmanueltouzery can you take a look at this? Otherwise I'll see if I can repro when I get a chance. @fengwk, could you provide some specific steps that reproduce the errors?

emmanueltouzery commented 1 year ago

Yes, I can try to look at it, but likely not before this weekend. Reproduction steps would be helpful for sure. Sorry for introducing a regression.

emmanueltouzery commented 1 year ago

So I could reproduce at least one of the issues simply by invoking :NvimTreeToggle three times -- show/hide/show.

And the issue is caused by overriding BufUnload. If I stick a return at the top of M.override_bufhidden = function(bufnr) the issue goes away. Which unfortunately means the issue is probably not going to be simple :(

emmanueltouzery commented 1 year ago

or to be more precise, trigger toggle twice (open, close), make sure less than 1s passes, and trigger a third time. nvim-tree expects the buffer to have been disposed of, but stickybuf overrode the bufhidden, and waits 1 second to clean up. That's at least one of the issues. triggering twice, waiting more than one second, and triggering a third time, works ok.

emmanueltouzery commented 1 year ago

so i think (and i have very limited understanding or both the stickybuf and nvim-tree codebases) that the problem is stickybuf's conservative approach "the buffer could be reused, so we'll wait one second to wipe it".

however it could be possible to inject filetype-specific knowledge, for instance, "nvim-tree for sure is not going to reuse buffers, so we don't have to wait one second in that case". However I'm not 100% sure it really won't reuse buffers, and even if it doesn't today, it might in the future, so.. :(

certainly changing the sleep time from 1000 to 0 fixes that one issue for me. But i'm not sure it's the only one.

fengwk commented 1 year ago

As emanueltouzery said, the problem in screenshot 2 was created after multiple executions of NvimTreeToggle, and screenshot 1 was created after the situation in screenshot 2 occurred :w.

emmanueltouzery commented 1 year ago

My assertion that nvim-tree probably does not reuse buffers is based on two observations:

  1. changing the timeout from 1000 to 0 seems to have no bad side-effects for nvim-tree (very quick testing)
  2. grepping for nvim_win_set_buf in the nvim-tree source code turns up only one use, and in that case the buffer that is set is created just before the call.
stevearc commented 1 year ago

We don't want the timeout to be 0, but it can probably be less than a second. I've changed it to 100ms and in my testing that's enough to not cause problems during normal use. You can still get the error if you bind "NvimTreeToggle" to a single keypress like <C-h> and holding the key down, but I think that's well outside the bounds of what a reasonable user would do.