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

User-defined tab rendering #35

Open
ap opened this issue Jul 28, 2017 · 9 comments
Open

User-defined tab rendering #35

ap opened this issue Jul 28, 2017 · 9 comments

Comments

@ap
Copy link
Owner

ap commented Jul 28, 2017

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.

@deathlyfrantic
Copy link
Contributor

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."

  1. g:buftabline_format string, possibly augmented by a g:buftabline_modified_symbol option. Example:
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.

  1. g:buftabline_format function callback, wherein the user can specify a function that takes a dictionary of options and returns a formatted string. Example:
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.

  1. g:buftabline_format list of dicts. Example:
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.

@ap
Copy link
Owner Author

ap commented Feb 19, 2018

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…)

@deathlyfrantic
Copy link
Contributor

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 g:buftabline_format variable, which should be a string that is the name of a function to use for rendering each tab. (I tried making this a funcref but function() and funcref() both require the assigned variable to be Uppercase, so let g:buftabline_format = function('MyFormatter') is not valid syntax.)

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 statusline - i.e. %#BufTabLineActive# or %3* (for User3) - that will be accounted for when calculating the width of the visible characters, so in theory you could highlight the modified symbol differently from the name of the buffer, etc.

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.

@ap
Copy link
Owner Author

ap commented Feb 28, 2018

Is there any point to that tab.opts key? The tab dictionary itself already has a num key – sticking an ordnum in there during the first loop is no problem. All the other data in the tab.opts dictionary is used only inside the callback function, which can just use getbufvar() and friends itself. So the sub-dictionary is redundant, the callback can just receive the tab itself.

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 tabs list, there would be no need to pass it the ordinal number either (it would just be the list index of a tab), and it could potentially control the overall set of tabs, to do things like hide certain tabs entirely.

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 %#BufTabLineFoo% highlighting escape instead of going into the label.

And trying to make that code work correctly is why I don’t want to take the approach you took with the s:visible_width function, of trying to accept an arbitrary 'tabline' format string and then trying to skip the escape codes inside it. For calculating the width that’s just about bearable (though it means the plugin must be updated every time Vim adds support for other escapes), but for modifying the label, that form is a bit of a nightmare.

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…

@ap
Copy link
Owner Author

ap commented Feb 28, 2018

If you want to keep working on this, do you want to do update #54 or close it and create a new PR?

@deathlyfrantic
Copy link
Contributor

Is there any point to that tab.opts key? The tab dictionary itself already has a num key – sticking an ordnum in there during the first loop is no problem. All the other data in the tab.opts dictionary is used only inside the callback function, which can just use getbufvar() and friends itself. So the sub-dictionary is redundant, the callback can just receive the tab itself.

There's no real "point" to the opts dictionary - flattening it into the tab dictionary would be fine - I mostly kept them separate for my own benefit while trying to unravel exactly how the rendering worked. I do think there is value in pre-calculating all the values of that dictionary, however, as it allows the rendering function to be pure, and also keeps that complexity out of it. My goal with this structure was keeping the rendering function as dumb as possible.

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 tabs list, there would be no need to pass it the ordinal number either (it would just be the list index of a tab), and it could potentially control the overall set of tabs, to do things like hide certain tabs entirely.

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.)

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 %#BufTabLineFoo% highlighting escape instead of going into the label.

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.

Maybe arbitrary highlighting needs to be punted to v2 of the API…

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).

If you want to keep working on this, do you want to do update #54 or close it and create a new PR?

Either is fine with me - I don't mind amending commits. If you have a preference just let me know.

@deathlyfrantic
Copy link
Contributor

I just pushed an amended commit that removes highlighting from the rendering function and flattens the opts dictionary into the tab dictionary. I kept most of the other changes intact for now.

@richin13
Copy link

richin13 commented Nov 3, 2019

Will this ever happen?

@deathlyfrantic
Copy link
Contributor

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.

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

No branches or pull requests

3 participants