vifm / vifm.vim

Vim plugin that allows use of vifm as a file picker
335 stars 19 forks source link

Files are opened in the wrong window on exit if files are deleted while using Vifm in a terminal #96

Closed rbong closed 4 months ago

rbong commented 4 months ago

Edit: may actually happen in other modes, not just embedded terminal mode

My init.lua:

require("pckr").add({
  {
    "vifm/vifm.vim",
    config_pre = function()
      vim.g.vifm_embed_term = 1
      vim.g.vifm_embed_split = 1
      vim.g.vifm_embed_cwd = 1
      vim.g.vifm_replace_netrw = 1
      vim.g.loaded_netrw = 0
      vim.g.loaded_netrwPlugin = 0
    end
  },
})

Steps to reproduce:

Expected result:

Actual result:

This is caused by fixes introduced for #85 which wipes out any buffers deleted while Vifm is open.

rbong commented 4 months ago

I would submit a PR to fix this, but there's a couple potential solutions we should discuss:

1) Add special logic to handle the case where a file has been deleted, Vifm is opened in an embedded terminal, and the file is opened in edit (default) mode (as opposed to split mode).

2) Remove the feature to delete buffers if the file has been deleted outside of Vim.

There are a few reasons I would argue for removal of this feature.

First, it's unintuitive. To have buffers you have open suddenly close when you exit Vifm is disorienting.

Second, it's counter to the way Vim usually handles removing files you have open in buffers. If you have the file open in a Window, Vim will let you know a buffer you are actively editing was deleted. If you have the file open in the background, Vim will let you know it was removed if you ever navigate back to the buffer. This makes it much more obvious what happened when a file is deleted, makes it possible to recover files, and is the method of handling deleted files every Vim user should be familiar with.

xaizek commented 4 months ago
  • Add special logic to handle the case where a file has been deleted, Vifm is opened in an embedded terminal, and the file is opened in edit (default) mode (as opposed to split mode).

I don't think special case is necessary. The issue seems to be a side-effect of layout change while closing those buffers. Currently they are closed before opening of picked file(s), but if they are closed after files are processed and new file replaced about-to-be-removed buffer in some window, the layout should remain uneffected.

  • Remove the feature to delete buffers if the file has been deleted outside of Vim.

Just make it configurable? If it will cause more issues, it could be turned off by default. Not that I depend on this behaviour, but Vim's way isn't super convenient either. When I switch branches in git and left with 10 buffers in several tabs that are no longer backed up by files, that's annoying because I can't even close so many manually.

rbong commented 4 months ago

I don't think special case is necessary. The issue seems to be a side-effect of layout change while closing those buffers. Currently they are closed before opening of picked file(s), but they are closed after files are processed and new file replaced about-to-be-removed buffer in some window, the layout should remain uneffected.

If you close buffers after opening selected files, be aware we will still have to handle the special edge case where:

To avoid not deleting a buffer with a file that the user explicitly wants to open. Either way we have to handle a special case, but I do think opening files first is better.

Just make it configurable? If it will cause more issues, it could be turned off by default. Not that I depend on this behaviour, but Vim's way isn't super convenient either. When I switch branches in git and left with 10 buffers in several tabs that are no longer backed up by files, that's annoying because I can't even close so many manually.

Making it configurable is fine. Just thought I would share my opinion on the feature because, arguably, messing with how Vim handles files that are deleted outside of it isn't Vifm's responsibility at all. I do understand if you want to keep this feature but I wanted to give a second opinion.

I do think that this behavior is already disorienting enough that it should be turned off by default, unless if it can be limited only to files that were deleted by Vifm and not just files that were deleted while Vifm was open (however I don't think that's possible since Vifm can open a shell).

I would mention I do like Vim's default deleted file handling behaviour, for example if I switch branches in Git and I have 10 files that are removed, I might be about to switch back to that branch in a minute. I think it's good that we have different opinions on this behaviour so we can balance out this feature, but I'm not sure how to weigh whether to make this default or not fairly without other users chiming in.

xaizek commented 4 months ago

If you close buffers after opening selected files, be aware we will still have to handle the special edge case where: [...]

If file that corresponds to a buffer exists, there is no reason to close the buffer, so no extra steps seem to be required.

I do think that this behavior is already disorienting enough that it should be turned off by default [...]

I'm fine with it being opt-in for those who want to have it (I'm not one of them, I just understand why someone might need it). I don't think it's a common case to move/delete files when you're trying to open another one.

rbong commented 4 months ago

If file that corresponds to a buffer exists, there is no reason to close the buffer, so no extra steps seem to be required.

In the case I'm describing, the file will not exist after closing Vifm, so the extra steps will be required. To clarify:

In Vim:

In Vifm:

The file will not exist because of !rm file_a. If we don't handle this case, the plugin will open and immediately close a file the user has asked to open, but that seems counter-intuitive.

I will work on a PR for this.