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

improve compatability with Goyo #67

Closed vigilancer closed 5 years ago

vigilancer commented 5 years ago

There is an issue with this plugin when used with Goyo.

This is how one can reproduce issue and test patch:

autocmd! User GoyoEnter nested call <SID>GoyoEnter()
autocmd! User GoyoLeave nested call <SID>GoyoLeave()

function! s:GoyoEnter() "
    "disable buftabline
    let g:buftabline_show = 0
    :call buftabline#update(0)
endfunction "

function! s:GoyoLeave() "
    "enable buftabline
    let g:buftabline_show = 2
    :call buftabline#update(0)
endfunction "

Actually issue have something to do with buftabline incorrectly populate tabline when Goyo mode is on. My fix just moves optimization if tabpagenr('$') > 1 | set guioptions+=e showtabline=2 | return | endif below if 0 == g:buftabline_show check. This ensures that let g:buftabline_show = 0 will always do what is expected from it - completely disable and hide buftabline.

ap commented 5 years ago

I am quite sure you have identified the right problem but I am doubtful that the fix is correct.

(The change to set showtabline to 0 instead of 1 smells very wrong – I suspect it ignores functionality that your configuration doesn’t require, and just makes the plugin work for your particular preferences. But that may be an unfair assessment; I won’t be able to say for sure until I have the code loaded into my head again.)

vigilancer commented 5 years ago

IIRC if tabpagenr('$') > 1 | set guioptions+=e showtabline=2 | return | endif line is kind of optimization that checks if there are more than one tab and then sets showtabline=2 ignoring g:buftabline_show option. Yes, this works for me and I have not heavily tested it with different configurations. Tend to agree that set showtabline=0 could be unnecessary in this fix, not tested it either. Glad that you on it.

ap commented 5 years ago

Well… this is quite a mess, really.

The tagpagenr('$') check is not optimisation but a feature that’s necessary because Buftabline doesn’t support Vim tabs at all. What it does is it allows the standard Vim tab feature to take over when the user or another plugin has created a Vim tab. Otherwise you wouldn’t see what’s going on.

But that means it mustn’t respect g:buftabline_show. The whole point of the check is that the creation of a Vim tab should override Buftabline, regardless of how Buftabline itself is configured.

That means your patch is, unfortunately, the wrong answer – for completely different reasons than I initially thought. Since there’s also an issue for this, I’m closing the PR.

Goyo collides with Buftabline because it breaks my underlying assumption that the user needs to be able to see what’s going on when a Vim tab is created. Goyo works by creating a new Vim tab that it wants to hide. This is… not a use case I had foreseen. It makes complete sense given Goyo’s aim, of course.

Not sure yet how I’m going to address it…