fix(files): avoid closing directory buffer when #windows decreases

[abeldekat]

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

mini_files_directory_buffer.mp4

Whilst working on this discussion, I saw inconsistencies with regard to the closing of the underlying directory buffer.

When decreasing the number of windows, there are circumstances where the directory buffer is closed.

-- Only added for visual feedback:
require('mini.tabline').setup()
require('mini.statusline').setup()

require('mini.files').setup()

It happens when 2 or more windows are closed.

Scenario:

• Open nvim
• Type :edit .
• Type ll
• Notice that the directory buffer is still open
• Type <
• Notice that the directory buffer is closed

The video demonstrates "depth_focus = 3", using both MiniFiles.refresh() and MiniFiles.trim_left(). The second part shows the same scenario with the solution applied.

In the source code, there are two locations where the behavior is triggered:

• refresh, explorer normalize
• refresh, after the loop

Unfortunately, I don't understand why BufEnter is triggered prematurely on the directory buffer. I assume it has to do with alternate files.

I fixed it by ensuring that in BufEnter the directory buffer is only explicitly closed if the explorer is closed.

[echasnovski]

Hmm... The behavior indeed seems strange, although I don't think it is harmful. Are there any actual problems that come from deleting the "directory buffer" early? Or is it only "doesn't feel right" kind of behavior?

I, of course, don't mind it fixing, only trying to understand the severity. Especially since it requires some extra checks that are not obviously needed.

[abeldekat]

There are no actual problems that come from closing the directory buffer early. It is surprising and a bit disorienting for the user, once he sees it happening.

In this discussion I added a full screen toggle, often triggering the behavior.

[abeldekat]

There is an alternative solution:

diff --git a/lua/mini/files.lua b/lua/mini/files.lua
index b92a2567..b2469d78 100644
--- a/lua/mini/files.lua
+++ b/lua/mini/files.lua
@@ -1572,6 +1572,7 @@ H.explorer_refresh = function(explorer, opts)
   -- Close possibly opened window that don't fit (like after `VimResized`)
   for depth = cur_win_count + 1, #explorer.windows do
     H.window_close(explorer.windows[depth])
+    vim.api.nvim_set_current_win(explorer.windows[cur_win_count])
     explorer.windows[depth] = nil
   end
 
@@ -1643,6 +1644,7 @@ H.explorer_normalize = function(explorer)
   -- still get outdated later if branch is too deep to fit into Neovim's width.
   for i = cur_max_depth + 1, #explorer.windows do
     H.window_close(explorer.windows[i])
+    vim.api.nvim_set_current_win(explorer.windows[cur_max_depth])
     explorer.windows[i] = nil
   end

[echasnovski]

There is an alternative solution:

Only by reading the diff, this seems very much too much: 1) it executes the same operation possibly several times; 2) setting current window might trigger relevant events, so should be done only when truly needed; 3) it looks like it might set current window to the last present window even if it is not the "true" current window.

[echasnovski]

I've taken a look at this and the issue is a bit more nuanced than the fix in this PR. A general rule of thumb is that current window should be set before closing any window. Otherwise closing the current window will temporarily switch focus to some other window (probably the first "normal" window in the tabpage).

Adjusting close() to postpone setting the current window after all explorer ones are closed somewhat follows the same pattern that lead to the current issue: it will guarantee to switch focus to some other window before setting the desired current window. In some cases it will be the same window, in some - not.

The more fundamental solution is to ensure that closing of a window in explorer never leaves the focus from explorer. I think it is achievable even with negative code diff without major consequences (although I might regret saying this; at least all tests pass). So I merged this in the temporary branch to adjust, as the test is mostly good.

[echasnovski]

Here is the commit that should fix this: f73ec14

---

I adjusted the test to have a bit less comments (they usually show the intent of codeblock/subtest or clarify some really non-obvious behavior) and use child.fn.bufnr(test_path) to find the target buffer (instead of relying that it is always vim.api.nvim_list_bufs()[1]).