ap / vim-buftabline

Forget Vim tabs – now you can have buffer tabs
http://www.vim.org/scripts/script.php?script_id=5057
MIT License
782 stars 75 forks source link

g:buftabline_show = 1 doesn't work when opening quickfix window #75

Closed deathlyfrantic closed 3 years ago

deathlyfrantic commented 3 years ago

When g:buftabline_show is 1, and there is only a single buffer open, opening the quickfix window displays the tabline, even though it shouldn't.

To reproduce:

  1. open vim/nvim
  2. :let g:buftabline_show = 1
  3. :copen

The tabline should not be visible, because the quickfix buffer shouldn't count as a visible buffer, but it is.

The root cause of this is here. There's nothing wrong with this code, but it's called from here during a BufAdd event, and as far as I can tell in both Vim and Neovim this event is fired before the buftype is set. So when the buftabline#user_buffers() code is being run, &buftype !=? "quickfix", the buffer is counted as visible, and showtabline is set to 2.

I don't even know if it's possible to fix this. I don't think there is an event analogous to BufAdd that gets run after the buftype is set. There's probably some hacky thing that could be done with timers and checking b:changedtick but it would be a mess.

I can work around it by doing :noautocmd copen, and this is probably the best that can be done short of having Vim/Neovim add e.g. a BufAddPost event.

It took me forever to track this down, so I'm mostly filing this issue so there is some record of it.

kentfredric commented 3 years ago

Side tip: if you use the "copy permalink" on the "..." left-side widget on github, it gives you a nice link that embeds the source line, but immune to getting content changes by subsequent commits :)

https://github.com/ap/vim-buftabline/blob/728b85835dcf965c67bbedeac02aed660ff3e07a/plugin/buftabline.vim#L44

https://github.com/ap/vim-buftabline/blob/728b85835dcf965c67bbedeac02aed660ff3e07a/plugin/buftabline.vim#L160

ap commented 3 years ago

It took me forever to track this down, so I'm mostly filing this issue so there is some record of it.

🙂 Relatable.

I don't even know if it's possible to fix this.

Don’t despair. 🙂

autocmd FileType qf call buftabline#update(0)

There is no event after the buftype is set, you are right.

And no other autocommands are run when that option gets set. (That function is not the only code that runs to set up a quickfix buffer – you can trace up through its callers. But the lack of autocommands is true for all of them.)

But there is an autocommand after filetype is set. No surprise to anyone, of course.

And Vim always does that for quickfix buffers. That is to say, it’s not conditional on a feature. (You can compile Vim without syntax support but not without filetype support.) Setting filetype ends up always happening only after setting buftype.

So the above autocommand is a reliable way to trigger an update of the tabline for new quickfix buffers at a point when buftype is guaranteed to have a value, and it avoids adding any more calls to buftabline#update() than absolutely necessary… or at least very nearly.


I feel flattered by this ticket, btw. Probably the best ticket I’ve ever gotten – a subtle, tricky bug, already thoroughly tracked down. (My own above excursion into Vim was a walk in the park, since you had already shown me exactly what to look for.)

deathlyfrantic commented 3 years ago

Thanks for the fix! I did briefly think about something like this - I was thinking about autocmd OptionSet qf call buftabline#update(0) - but for some reason assumed adding another call to update was verboten. 🤷‍♂️

ap commented 3 years ago

You mean autocmd OptionSet buftype I guess?

You aren’t entirely wrong about adding more calls to update – I wouldn’t have wanted to trigger tons of additional calls just to be able to catch this case. Even without having had to, I am very slightly bothered by the untidiness of the fix requiring update to run twice for quickfix buffers… but it doesn’t bother me materially. Because fundamentally this fix isn’t sloppy.

The most sloppy would have been to simply cause update to be called all the time to paper over the problem by coincidence, of course.

The OptionSet route feels somewhere in between – it would address the problem instead of papering over its effect, but still, that event is triggered a lot more often than FileType. And OptionSet buftype matches more often than FileType qf does, too, so the autocommand would run more often. So the overhead would be rather higher that way. If that were the only choice available, I would still have considered it and probably gone for it – it’s not like the slowdown would be measurable, and there is a bug to fix here. But having that fix in the code would keep bothering me afterwards.

The FileType approach OTOH is quite close to optimal. It’s unfortunate that the amount of calls to update is more than the exact amount necessary – I’d prefer an exact solution. But it’s off by only a tiny and very predictable number: creating a quickfix buffer triggers an extra call to update. And an exact solution is not possible currently, and would in any case require a new version of Vim with quite a bit of internals surgery, which is way over-the-top effort just to get rid of this one extra call. So I can live with this fix just fine.