tpope / vim-fugitive

fugitive.vim: A Git wrapper so awesome, it should be illegal
https://www.vim.org/scripts/script.php?script_id=2975
19.82k stars 1k forks source link

Option to disable winfixbuf. #2286

Closed adriantrunzo closed 5 months ago

adriantrunzo commented 5 months ago

Please feel free to resolve this issue as user error. I acknowledge that I can rollback to a previous version of vim-fugitive to resolve my issue.

The recent addition of winfixbuf and its usage in vim-fugitive is causing me the following pain point -- I keep ending up in a state where I am looking at Vim with one tab, one window, and one blank buffer, but I can't open any buffers. Here are the reproduction steps:

  1. Open vim without any arguments in a git repository, just vim.
  2. :Git
  3. C-W C-W to switch to the not fugitive split.
  4. :bd
  5. Now your back in the fugitive window.
  6. :bd
  7. Try to open a new buffer.

https://github.com/tpope/vim-fugitive/assets/1218900/67641085-8a83-4c02-a3b5-e16d05e9701a

I never deliberately follow this exact sequence, but I think that at some point during the day I end up closing the fugitive buffer with :bd and it happens to be in my only remaining window.

I've always treated the fugitive buffer/window as just another buffer and never ran into any issues. My ideal fix would be the option to disable the use of winfixbuf altogether, at least just in vim-fugitive. Does anybody have other suggestions?

Two workarounds I've identified when I am in this state:

adriantrunzo commented 5 months ago

I'm happy to go with the suggestions mentioned in https://github.com/tpope/vim-fugitive/issues/2272#issuecomment-2021506266 and https://github.com/tpope/vim-fugitive/issues/2272#issuecomment-2021539581.

Thinking about this more, maybe the issue is that winfixbuf allows you to close the buffer with :bd without a warning or error?

I suppose the Vim behavior I want is that If you happen to :bd the buffer in a winfixbuf window and that's the only remaining window, it should automatically unset winfixbuf. It's not clear to me how winfixbuf is still useful in that scenario when it's on the only remaining window with an empty buffer.

tpope commented 5 months ago

@ColinKennedy thoughts? Almost seems like something that should be accounted for in :bd?

ColinKennedy commented 5 months ago

This is a really interesting case. And one that I wouldn't normally encounter because I tend to :close over :bdelete.

Since 'winfixbuf' is a pairing of window-to-buffer, calling :bdelete I think needs to either

  1. fail to delete the buffer because a window is 'winfixbuf'-ed to it
  2. it could unset 'winfixbuf' on all windows that point to the buffer

Another way of thinking about this could be

  1. Do we enforce the window->buffer connection (E1513 error)
  2. Or act like no connection ever happened (because the buffer doesn't exist anymore)

That rephrasing helped me because, from it, a couple more options come up

  1. Prevent :bdelete and error with E1513 b. Unless it's :bdelete!, in which case ...
    • Delete the buffer and unset 'winfixbuf' on all windows that point to it
  2. Allow :bdelete but don't unset 'winfixbuf' (which is how things are, currently)

Between the 4 options, I'm partial to (3). It's in the spirit of the original but also gives the most flexibility for others. My only worry about (3) is if we make :bdelete! affect all windows then the user won't be able to express "only unset 'winfixbuf' on this one window. They'd have to handle all that via vimscript themselves. But that case should be pretty niche case, right?

No matter which way we go, as long as it's not (4), I'll need to update all commands that modify / delete buffers. That means :bdelete, :bwipeout, possibly also :bunload. I'd have to look into those a bit more

ColinKennedy commented 5 months ago

Update: It sounds like we're going with (3). Which is

Prevent `:bdelete` and error with `E1513`
    b. Unless it's `:bdelete!`, in which case ...
    - Delete the buffer and unset `'winfixbuf'` on all windows that point to it

@adriantrunzo if that isn't suitable for you then please let us know. I think that it will though!

tpope commented 5 months ago

This is now off by default, at least until the option stabilizes. To opt back in:

autocmd User FugitiveIndex let &winfixbuf = exists('w:fugitive_status')
adriantrunzo commented 5 months ago

@tpope Thanks for your time. I'll update to the latest version.

@ColinKennedy I think your proposal is reasonable given the intent of winfixbuf. Seeing an error is better than ending up in a confusing state. In all of my years using Vim, I've never had to pay attention to all of the different ways you can close buffers and windows. I am not a heavy user of splits and separate windows and I map a key (<Backspace>) to :bd. I imagine that I can locally map that key to :bd! in the plugin buffers that set winfixbuf and I won't notice the difference.