-
Notifications
You must be signed in to change notification settings - Fork 72
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
User-defined tab rendering #35
Comments
Continued from #52! I have some thoughts about how this could be implemented. I have three different ideas that range from "simple but not super configurable" to "wildly configurable and batshit complicated."
let g:buftabline_modified_symbol = '+'
let g:buftabline_format = '{mod}{num} {name} [{bufnum}]' Pros: pretty configurable, conceptually simple. Cons: not as configurable as options 2 or 3.
function! MyBuftablineFormatter(opts)
let mod = a:opts.modified ? '+' : ''
return printf('%s%s %s [%s]', mod, a:opts.number, a:opts.name, a:opts.bufnum)
endfunction
let g:buftabline_format = function('MyBuftablineFormatter') Pros: possibly simpler than number 1 from a code perspective, though there would have to be a default included so as to provide backward compatibility; allows for as much configuration as the user wants. Cons: more difficult to configure for novice users, possibly over-complicated.
let g:buftabline_format = [
\ {'name': 'modified', 'format': '%s', 'symbol': '+'},
\ {'name': 'number', 'format': '%2s'},
\ {'name': 'name', 'format': '%15s'},
\ {'name': 'bufnum', 'format': '[%2s]'}
\ ] Pros: super configurable (allows for printf-style strings to control field widths), arguably less complicated for users than number 2[?]. Cons: way too verbose, code would be too complex and I don't even really like this idea. But I think it roughly reflects the configuration style of some other plugins, e.g. I think Lightline is configured like this (or at least was last time I looked at it). If you like any of these ideas and want me to take a shot at implementation just let me know! Though I totally understand if this is something you want to handle yourself. |
Option 2 is what I was thinking of. To some degree it wouldn’t even be any more difficult for novices than using the current options, since the backcompat fallback would work exactly the same as the code that is there now, and would be just as easy to configure. But there is indeed a steep cliff of difficulty if they want to customise the display in a way that’s outside the boundaries of the built-in configuration options. Options 1 and 3 I dislike because they require a lot more code to implement than what’s there already, and yet the resulting interface still has very limited expressive power, and on top of all that, the backcompat story requires another layer of ugly code on top. It‘s pretty clear it would have to be a callback. The reason I’ve been dragging my feet for so long is twofold. One is the aforementioned cliff; I wonder if I can think of a way to alleviate that somehow. The other is that there is very little room for mistakes. Adding a callback requires defining some kind of interface (a signature; data structures; whatever) for passing information back and forth… and once users start writing code against that interface, it’ll be very hard to change. So it needs to be designed carefully. And the big problem there is my lack of incentive. I don’t know what a good design would look like, because I’d have to try using it in anger in order to figure out what needs that interface must accommodate. But… I don’t have an incentive to use such an interface myself: the default styling of the plugin is designed to best fit my own preferences after all. There’s no itch for me to scratch. And so the question has been languishing in limbo for years. The tricky part is that I’d like to enable users to use multiple highlight groups in a single tab label, unlike the built-in code. But then I have to have a way to measure their width and truncate them – without screwing up the display. There’s also the question of whether and how to open up the disambiguation algorithm to customisation. (I’m not entirely done putting down my thoughts but I have to cut the writing short at this point for now…) |
I created a PR (that I do not expect you to merge as-is) - #54. Basically this duplicates the existing functionality, but allows for setting a The formatter function is passed a dictionary of options and should return a formatted string. The formatter function can include arbitrary highlight groups a la There could be (probably are?) bugs here. I haven't done extensive testing, but I did open a bunch of files and make sure the centering still works with multiple highlight groups, etc. Let me know if this seems like it's going the right direction. I personally find this a bit easier to follow since the rendering code is separated from the "data accumulation" code, but that could just be an "I wrote this so I understand it" bias. |
Is there any point to that I don’t think this is the right factoring though. The callback is called once per tab; OT1H that is nice for keeping the callback focused on rendering and keeping control over the overall control flow. OTOH if the callback was passed the whole I’ve also had feature requests for modifications to how the label is derived from the file path, e.g. to always show at least one containing directory. Or someone might like the disambiguation to work a little differently. (The current algorithm is less than great. It’s just simpler and less bad than several other ideas I tried.) For being able to do at least some of these things, it’s necessary to pass the entire list. Maybe these should be separate callbacks, though… (Also, this would mean it would be just one single function call, no matter how many buffers there are. (Though, I guess it would force the function to run a loop, whereas the repeated calls happen inside a loop in the plugin… and I don’t know how expensive function calls and loop iterations are relative to each other, so I don’t actually know which one would scale better. Needs benchmarking…)) Another thing, going a little bit in the opposite direction: it’s probably not ideal to outsource the highlighting to the callback so completely. It means that the user who wants custom rendering has to write code to emulate the default highlighting. I’m inclined to have the outer highlighting still controlled by buftabline, so the callback doesn’t have to take care of that. (Of course if the callback adds its own highlighting, that will take precedence. So it still has full control over the rendering.) Related to that – it seems to me, though I’ve not confirmed it, that you’ve created a bug, in that the code which clips tabs at the ends of the screen will cut characters from the And trying to make that code work correctly is why I don’t want to take the approach you took with the The problem is I don’t have any great ideas for what to use instead. The only idea I do have is kinda sucky; the idea being that if you want to put your own escapes in the label, you return the label as a list rather than a string, where escapes and literal strings alternate. Then the clipping code can cut characters from just the odd-index elements, but at the end it would join together the entire list. That’s a fairly awful interface from the user’s pespective though, and it requires even more awful code on the plugin’s end to process… nothing like the simplicity of the current clipping code. Maybe arbitrary highlighting needs to be punted to v2 of the API… |
If you want to keep working on this, do you want to do update #54 or close it and create a new PR? |
There's no real "point" to the
I see your point here but if we pass a list of tab data to the rendering function, what is it supposed to return? A list of formatted strings? Certainly returning a fully-formatted single string is not going to work, because then the plugin has no concept of where tabs are and would not know how to space or clip them. As far as hiding tabs goes, the current PR allows for that (I think - haven't actually tested) by returning an empty string. My feeling is passing a list of tabs to the rendering function may make sense for the default rendering function that lives in the plugin, but becomes problematic for user-defined rendering functions. I think from a simplicity perspective, having the render function take a dictionary of data about a single tab and return a formatted string as the label for that tab is the most intuitive behavior. Given this is meant to be user-configurable, I think keeping the interface as simple as possible is desirable, even if it might be marginally slower. (But then again, anyone configuring this will have to know a fair amount of Vimscript, so maybe weighing simplicity so heavily is misguided on my part.)
I am sure you are correct - this is not something I considered. I have an idea how to work around this, but it would be a huge mess, so I will probably just move the highlighting back into the plugin. I tacked on the highlighting stuff right at the end, hoping to close a couple of the open issues with that feature, but as you point out, it's very problematic to the say least.
I think this is probably the best idea. Let's limit this round to providing users the ability to customize the "shape" of the tab - i.e. the contents of the string itself - and worry about arbitrary highlighting later (or never).
Either is fine with me - I don't mind amending commits. If you have a preference just let me know. |
I just pushed an amended commit that removes highlighting from the rendering function and flattens the |
Will this ever happen? |
Personally I have forgotten why I wanted to do this in the first place so I'm unlikely to put any effort into driving it forward. My initial attempt is in #54 if anyone else wants to build on it. |
To avoid cramming everything possible into configuration options, allow the user to specify how tabs should be rendered, possibly by providing custom rendering code for tabs.
The text was updated successfully, but these errors were encountered: