echasnovski / mini.nvim

Library of 40+ independent Lua modules improving overall Neovim (version 0.8 and higher) experience with minimal effort
MIT License
4.45k stars 171 forks source link

fix(files): make `window_focus` setup correct win_id for current buffer #948

Closed wenjinnn closed 3 weeks ago

wenjinnn commented 3 weeks ago

When setup config.windows.preview = true, and go in to directory depth that make maximum number of window width exceeds nvim width, the preview window will not refresh when CursorMoved triggered.

Here's a show case: mini files-bug

After some dig I find out that in this case, when H.explorer_refresh_depth_window and H.window_close called in H.explorer_refresh, the buffer on current window will lost their win_id.

echasnovski commented 3 weeks ago

Thanks for the PR!

Based on the description, I was able to reproduce this behavior only once. There seems to be some other factors should take place. This also might be the same issue reported in #858.

Nevertheless, I don't think registering inside window_focus() that buffer is shown in the window is a good place to do that. If anything, doing that in window_open() is a better place, but I'd have to find a reproducible example first to make sure.

wenjinnn commented 3 weeks ago

Thanks for reply!

I can help with a reproducible example. It only happens once in this case, after trigger this bug, if you press hl, which reenter the target directory, the behavior becomes normal.

If you adjust your nvim window to smaller that can only display two minifiles window, then it will be easier to reproduce. mini files-bug2 And some other factors might be related to this variable, https://github.com/echasnovski/mini.nvim/blob/d330f2639462084d2ef6c699ccd6219b81c45bc7/lua/mini/files.lua#L1315 In my opinion, the win_id was mistakenly set to nil in the following for loop because this wrong value of depth_range.to.

echasnovski commented 3 weeks ago

Thanks for the tips! I was already able to reproduce this the hard way.

It is indeed must be related to how visible depth range is computed, because I could not reproduce with overall width 85 but could reproduce with width 86 (which is just enough to show focused and two nonfocused windows). I'll investigate further.

echasnovski commented 3 weeks ago

I think I found the culprit. Whether to show preview depends on the current buffer, yet in some cases buffer is set after that check is done. It currently mostly works due to explorer state being re-synched in CursorMoved event, but also results in a slight flicker.

And apparently it breaks in some situations. Like during computation of visible range which has extra check for preview (focused depth should not be last, as preview is to the right), but in some cases it is not yet true (as no preview is yet registered to be shown).


@wenjinnn, would you mind applying the following patch and try if it fixes the issue for you?

diff --git a/lua/mini/files.lua b/lua/mini/files.lua
index 8e3dd072..75b8ca24 100644
--- a/lua/mini/files.lua
+++ b/lua/mini/files.lua
@@ -1416 +1416 @@ H.explorer_sync_cursor_and_branch = function(explorer, depth)
-  local is_cur_buf = buf_id == vim.api.nvim_get_current_buf()
+  local is_cur_buf = explorer.depth_focus == depth
wenjinnn commented 3 weeks ago

@wenjinnn, would you mind applying the following patch and try if it fixes the issue for you?,您介意应用以下补丁并尝试它是否可以解决您的问题吗?

diff --git a/lua/mini/files.lua b/lua/mini/files.lua
index 8e3dd072..75b8ca24 100644
--- a/lua/mini/files.lua
+++ b/lua/mini/files.lua
@@ -1416 +1416 @@ H.explorer_sync_cursor_and_branch = function(explorer, depth)
-  local is_cur_buf = buf_id == vim.api.nvim_get_current_buf()
+  local is_cur_buf = explorer.depth_focus == depth

LGTM! I think this solved the real problem, preview now works fine.

echasnovski commented 3 weeks ago

Thanks for the confirmation! And for narrowing down when this issue happened!

This now should be fixed on latest main.