fix(files): remove invalid window from explorer.windows

[abeldekat]

• I have read CONTRIBUTING.md
• I have read CODE_OF_CONDUCT.md

Details:

• When MiniFilesWindowUpdate uses get_explorer_state() with enabled preview, an error occurs after the user presses o, <Esc>, u.

The error:

Error in User Autocommands for "MiniFilesWindowUpdate":                                                                                                                                      
Lua callback: ...m-repro/site/pack/vnr/start/mini.nvim/lua/mini/files.lua:1105: Invalid window id: 1004                                                                                      
stack traceback:                                                                                                                                                                             
        [C]: in function 'nvim_win_get_buf'                                                                                                                                                  
        ...m-repro/site/pack/vnr/start/mini.nvim/lua/mini/files.lua:1105: in function 'get_explorer_state'                                                                                   
        /home/user/.config/nvim-repro/init.lua:12: in function </home/user/.config/nvim-repro/init.lua:12>                                                                                 
        [C]: in function 'nvim_exec_autocmds'                                                                                                                                                
        ...m-repro/site/pack/vnr/start/mini.nvim/lua/mini/files.lua:3096: in function 'trigger_event'                                                                                        
        ...m-repro/site/pack/vnr/start/mini.nvim/lua/mini/files.lua:1873: in function 'explorer_refresh_depth_window'                                                                        
        ...m-repro/site/pack/vnr/start/mini.nvim/lua/mini/files.lua:1566: in function 'explorer_refresh'                                                                                     
        ...m-repro/site/pack/vnr/start/mini.nvim/lua/mini/files.lua:2147: in function 'fn'                                                                                                   
        [string "vim/_core/editor"]:408: in function <[string "vim/_core/editor"]:407>    

Repro:

require('mini.files').setup({ windows = {  preview = true } })
vim.api.nvim_create_autocmd('User', {
  pattern = 'MiniFilesWindowUpdate',
  callback = function() MiniFiles.get_explorer_state() end,
})

1. Open nvim
2. Type :lua MiniFiles.open()
3. Type o and press <Esc>.
4. Type u

This happens because the preview buffer is deleted. However, the win_id is still present in explorer.windows.

[echasnovski]

In theory, this is not the cleanest assignment as it can introduce "holes" in the array (i.e. explorer.windows[2] = nil while explorer.windows[3] is not nil).

My first instinct tells me that the culprit is probably somewhere else.

[abeldekat]

My first instinct tells me that the culprit is probably somewhere else.

Interesting.
When I comment this line, the error does not happen.

[abeldekat]

This behavior could potentially happen starting from this commit.
I think it wasn't noticed because the examples don't use get_explorer_state in MiniFilesWindowUpdate. I found it when playing with the code example from the custom side scrolling discussion

[abeldekat]

In theory, this is not the cleanest assignment as it can introduce "holes" in the array (i.e. explorer.windows[2] = nil while explorer.windows[3] is not nil).

I fixed that.

[echasnovski]

Thanks for the PR!

I can reproduce. And not only after u (which might indicate on some undo related issues), but also after moving cursor to the file that is present on disk.

[echasnovski]

After a more research, it looks like the culprit is that the events are triggered too early during the "explorer refresh" cycle. This makes code a bit cleaner, but has a downside of making get_explorer_state() act on not yet fully refreshed explorer state. The error also happens in MiniFilesWindowOpen event.

As usually triggering events when everything is up to date (be it "already up to date" for open/changed kind of events or "yet up to date" for close/changedPre kind of events) is a good idea, triggering both problematic events a bit later seems okay. This also shows that all the bookkeeping about the explorer state was correct.

Thanks again for finding this and making a PR!