fix(pick): prevent invalid buffer when opening mini.files

[abeldekat]

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

Fixes: #2156

I think it's a timing problem, as the error does not occur when the starter is visible.
It's very strange though: The window is valid. I tested that even the buffer is valid. Still, nvim_set_current_win crashes.

[echasnovski]

Thanks for the PR!

Although this technically fixes the #2156, I'd ask if you want to find the culprit here. As this is very strange to expect errors after explicitly checking that win_id is valid.

Writing a test case would possible make this easier. My guess is that this has to do with 'mini.files' being default explorer.

[abeldekat]

Yes, without mini.files the problem does not occur.

My guess is that mini.files is "in progress", creating or refreshing its buffers. I just don't understand why it doesn't happen when mini.starter is visible.

I will look into it.

[abeldekat]

I modified mini.files a little to prevent an explicit vim.api.nvim_buf_delete:

H.track_dir_edit = function(data)
  -- Make early returns
  if vim.api.nvim_get_current_buf() ~= data.buf then return end

  if vim.b.minifiles_processed_dir then
    -- Smartly delete directory buffer if already visited
    local alt_buf = vim.fn.bufnr('#')
    if alt_buf ~= data.buf and vim.fn.buflisted(alt_buf) == 1 then vim.api.nvim_win_set_buf(0, alt_buf) end
    return -- COMMENTED return vim.api.nvim_buf_delete(data.buf, { force = true })
  end

  local path = vim.api.nvim_buf_get_name(0)
  if vim.fn.isdirectory(path) ~= 1 then return end

  -- Make directory buffer disappear when it is not needed
  vim.bo.bufhidden = 'wipe'
  vim.b.minifiles_processed_dir = true

  -- Open directory without history
  vim.schedule(function() MiniFiles.open(path, false) end)
end

I added some print statements. The scenario: Open nvim with init.lua, press fv, select visited directory entry in picker, enter...

mini.files in track dir for data.buf 1
mini.files in track dir, no early return
mini.files in track dir, return, not a directory /home/user/.config/repro/init.lua
mini.pick in default chose
mini.files in track dir for data.buf 3
mini.files in track dir, no early return
mini.files still in track dir, marking vim.b.minifiles_processed_dir
mini.pick advance before stop -- NOTE: mini.files does not yet appear....
mini.pick set curwin
mini.files in track dir for data.buf 3
mini.files in track dir, no early return
mini.files in track dir, has vim.b.minifiles_processed_dir, alt buf is 1
---
--- Without modification, crashes here...
---
--- Happy path:
mini.files in track dir for data.buf 4
mini.files in track dir, no early return
mini.files in track dir, return, not a directory minifiles://4//home/user/.config/repro

The data.buf 3 must be the mini.files buffer. The problem occurs after mini.pick sets the current window in its picker_stop. As a result, mini.files H.track activates again, this time with vim.b.minifiles_processed_dir set. Without the modification, data.buf 3 is actively deleted... This also explains why there is no error when the dashboard is active. That buffer is unlisted...

So, the fix could also be in mini.files, by not deleting the buffer and just return instead. Perhaps the explicit delete is not necessary anyways, because of vim.bo.bufhidden = 'wipe'

I think its best to fix in mini.files, to not crash on an external vim.api.set_current_win. All tests pass.

[echasnovski]

So, the fix could also be in mini.files, by not deleting the buffer and just return instead. Perhaps the explicit delete is not necessary anyways, because of vim.bo.bufhidden = 'wipe'

I think its best to fix in mini.files, to not crash on an external vim.api.set_current_win. All tests pass.

The buffer deleting is for the following use case:

• :edit . (or any other directory).
• Press q. Without deleting there is a buffer for the current directory if there is no alternative buffer. Removing nvim_buf_delete() leaves the buffer for the target directory.

It is probably not a big deal and a rare use case, but at least trying to account for that is reasonable.

And, of course, needs tests both here and 'mini.extra'.

[abeldekat]

I unfortunately cannot reproduce the buffer deleting scenario you described. Testing with ls! I don't see dangling buffers after the fix.

Still todo: A test in mini.extra

[echasnovski]

I unfortunately cannot reproduce the buffer deleting scenario you described. Testing with ls! I don't see dangling buffers after the fix.

1. Create '~/.config/nvim-repro/init.lua':

   -- Clone latest 'mini.nvim' (requires Git CLI installed)
   vim.cmd('echo "Installing `mini.nvim`" | redraw')
   local mini_path = vim.fn.stdpath('data') .. '/site/pack/deps/start/mini.nvim'
   local clone_cmd = { 'git', 'clone', '--depth=1', 'https://github.com/nvim-mini/mini.nvim', mini_path }
   vim.fn.system(clone_cmd)
   vim.cmd('echo "`mini.nvim` is installed" | redraw')
   
   vim.cmd('packadd mini.nvim')
   
   require('mini.files').setup()

2. Apply the following patch to '~/.local/share/nvim-repro/site/pack/deps/start/mini.nvim/':

   diff --git a/lua/mini/files.lua b/lua/mini/files.lua
   index 470cb20..ace4fc6 100644
   --- a/lua/mini/files.lua
   +++ b/lua/mini/files.lua
   @@ -1383,7 +1383,7 @@ H.track_dir_edit = function(data)
        -- Smartly delete directory buffer if already visited
        local alt_buf = vim.fn.bufnr('#')
        if alt_buf ~= data.buf and vim.fn.buflisted(alt_buf) == 1 then vim.api.nvim_win_set_buf(0, alt_buf) end
   -    return vim.api.nvim_buf_delete(data.buf, { force = true })
   +    return
      end
    
      local path = vim.api.nvim_buf_get_name(0)

3. NVIM_APPNAME=nvim-repro nvim

4. :edit . and press q.

5. :ls shows a buffer named ., although it should not be there. To make it more visible, the previous step can be :edit ~/.config/nvim-repro/.