willothy / flatten.nvim

Open files and command output from wezterm, kitty, and neovim terminals in your current neovim instance
https://luarocks.org/modules/willothy/flatten.nvim
MIT License
497 stars 13 forks source link

feat: get buffer id from the open function #46

Closed sassanh closed 1 year ago

sassanh commented 1 year ago

The code to guess the list of added buffer ids was a bit hacky and didn't work all the time, it could return nvim-notify buffers, or not return the buffers that were already present in the buffer list for example. This code ran only when the open option was set to a lua function.

This PR tries to fix this issue by making the user return the buffer id as they have more context about how their open function handles the buffers.

For example most of the time I open terminals in floating windows, I have a custom open function which opens the buffer in the background main window like this:

          open = function(buffer_ids)
            local window_id = vim.api.nvim_tabpage_list_wins(0)[1]
            for _, buffer_id in ipairs(buffer_ids) do
              vim.api.nvim_win_set_buf(window_id, buffer_id)
            end
          end

It didn't work most of the time as buffer_ids were not correct and it also disabled block_for because winnr and bufnr were incorrect in flatten's code resulting in an empty ft which was not detected as git. So most of the time git commit resulted in "Aborting commit due to empty commit message.".

Now after this PR I can change it to something like this:

          open = function(files)
            local window_id = vim.api.nvim_tabpage_list_wins(0)[1]
            local buffer_id
            for _, file in ipairs(files) do
              buffer_id = vim.fn.bufnr(file)
              vim.api.nvim_win_set_buf(window_id, buffer_id)
            end
            return buffer_id
          end

and everything works. In case the user's workflow involves opening buffers that vim.fn.bufnr(file) doesn't work for them, they know the context, the reason why it doesn't work, what's special about their buffers and can better handle it in their open function, I think it is a better approach than us trying to have a general solution in flatten.core to blindly predicting all the special buffer types/files different users may try to open with flatten.

P.S. I will add a commit to update the docs.

sassanh commented 1 year ago

Unfortunately it is a breaking change, we may try to make it backward compatible by:

  1. Send files as the third argument of the callback or as an array in the files key of the first argument, the buffer_ids.
  2. Fallback to buffer_ids[1] when open returns nothing.

But my understanding is that considering the user base of flatten.nvim is not very big at the moment, and probably a small fraction of them uses open = function option (otherwise we would have some reported issues about it), It shouldn't harm if we have this as a breaking change.

willothy commented 1 year ago

Thanks! Yeah, I wasn't too happy with the code to guess the new buffers either, and I'm alright with it being a breaking change. I'll look through the changes and test this out later today :)

sassanh commented 1 year ago

@willothy I thought maybe you are busy so I went ahead and updated the docs. Feel free to change it if needed.

willothy commented 1 year ago

@willothy I thought maybe you are busy so I went ahead and updated the docs. Feel free to change it if needed.

Thanks, I appreciate it! Yeah, sorry about that, I was quite busy this week. Merging now :)