Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow disabling the Buftabline #72

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Allow disabling the Buftabline #72

wants to merge 5 commits into from

Conversation

jandamm
Copy link

@jandamm jandamm commented Feb 1, 2020

I really like Buftabline but I want to use my Tabline for more than just buffers.
So I implemented a way to disable Buftabline but still be able to use it in my code.

Changes:

  • enable autoloading
  • disable multiple sourcing of files
  • add 'g:buftabline_enabled' (which defaults to true)

What does this achieve:

The user has far more flexibility about the layout of the Tabline.

function RightAlignTabline()
    return '%=' . buftabline#render()
endfunction

Or just create your own Tabline with a greeting and showing the current working dir.

function MyTabline()
    return 'Hi ' . buftabline#render() . '%=%{getcwd()}' . 
endfunction

In contrast to the full disable toggle, this links highlight groups and
defines default values but does not enable autocmds of Buftabline.
@ap ap added the enhancement label Feb 2, 2020
@ap
Copy link
Owner

ap commented Feb 2, 2020

The basic idea is reasonable.

But this PR seems insufficiently ambitious for that. Buftabline does its rendering based on &columns i.e. it’s written with the assumption that its output is going to exactly take up the whole tabline. If you want to use it as only a component of the tabline, how would that work? Doesn’t that issue require some kind of solution, such as passing the width as an argument? (Which would then necessitate changes to keep the signature of render() identical for backcompat…) And that seems like the bare minimum… it would require allocating a precomputed amount of space to the buftabline so it would not allow sizing other components dynamically based on its actual size (such as if there are only a few tabs, and you have more room for other components).

As to the individual changes:

  • Is there any point to lazy-loading buftabline? It’s not a ton of code and doesn’t spend any time initialising itself. The only real gain I can see would be to reduce the number of files sourced at startup… which that commit doesn’t do. It does however add a file to be sourced later…

  • Is the variable to prevent re-sourcing an autoload plugin actually needed? Under what circumstances would an undesirable re-sourcing happen? (And if it is in fact needed, then it only becomes another downside of autoloading…)

  • The loaded variable is fine.

@jandamm
Copy link
Author

jandamm commented Feb 2, 2020

The now has a way to fully disable it. Autoloading allows
A) to use the functions while disabled
B) the tabline code doesn't have to be loaded in every session
C) the function name indicates autoloading which I found confusing

But you're right, it doesn't really speed up the startup time, it was more of a personal preference.

The autoloaded would be needed in case a not existing function or a user function with the same namespace would be called.

@jandamm
Copy link
Author

jandamm commented Feb 2, 2020

I've got a very wide screen so until now I didn't have more tabs than my screen can show. I'll think if I can find a good solution for this issue 👍

@jandamm
Copy link
Author

jandamm commented Feb 2, 2020

I think the best option is to have the user provide the space for the rendering:

function! buftabline#render()
   return buftabline#renderWithWidth(&columns)
endfunction

function! buftabline#renderWithWidth(width)
" ...
endfunction

My previous example would then be:

function MyTabline()
    let l:cwd = getcwd()
    return 'Hi ' . buftabline#renderWithWidth(&columns - 3 - len(l:cwd)) . '%=' . l:cwd
endfunction

set tabline=%!MyTabline()

I would implement this into my PR if the naming is fine for you.

Should I revert the autload?
Personally I'd prefer it this way, as I like this structure when looking at a plugin (seeing where the functions are and where/what is actively called by the plugin). But I can see your point of an "one file plugin" 👍

@jandamm
Copy link
Author

jandamm commented Feb 26, 2020

Hi @ap, I've changed the implementation of the render function while still being backwards compatible.
I also removed autoloading as I understand that your focus is to have a single file plugin 🎉

@tbremer
Copy link

tbremer commented Mar 1, 2021

Hey, just wanted to say that I really enjoy this plugin, and I came here to open an issue around this idea. Is there anything holding it up from being merged?

@jandamm
Copy link
Author

jandamm commented Mar 3, 2021

@tbremer not from my point of view. I've used my fork for a while and the stopped using buftabline altogether. (Using the default now).
Haven't had an issue using it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants