tabline.nvim icon indicating copy to clipboard operation
tabline.nvim copied to clipboard

Fix the `allowed_buffers` table

Open s-cerevisiae opened this issue 3 years ago • 2 comments

Originally the allowed_buffers table of current tab, if created with :TablineTabNew, is a set with filepath as its keys and true as values, but if created with :TablineBuffersBind, it would be a list with filepath as its values. And the contains function tests for the list-like behavior, so :TablineTabNew has some weird behavior when called with multiple files.

This PR changes M._bind_buffers() and removes contains() to ensure that allowed_buffers is always a set-like structure and is tested with indexes.

I'd also like to ask if it's better to store bufnr (which could be get with bufname()) instead of filepath in allowed_buffers.

PS: Thank you for creating this plugin. It's embarrassing that I didn't know the "tab" part is so useful until today...

s-cerevisiae avatar Apr 20 '22 16:04 s-cerevisiae

Thanks so much for the PR! Yeah, I think you are right about that, I didn't test that extensively when that feature was added.

I'd also like to ask if it's better to store bufnr (which could be get with bufname()) instead of filepath in allowed_buffers.

I think a unique handle would better. Somewhat related: https://github.com/kdheepak/tabline.nvim/issues/21

PS: Thank you for creating this plugin. It's embarrassing that I didn't know the "tab" part is so useful until today...

haha yeah it's also not very well documented here. I think this could be a lot more useful and powerful too, but I'll have to wait a little while before I can find time to implement some other ideas I have.

kdheepak avatar Apr 20 '22 16:04 kdheepak

I think a unique handle would better.

To my knowledge, unlike tab number, bufnr is not changing once the buffer is created, so it serves as the unique handle of a buffer. Please correct me if that isn't the case.

Edit: I found it non-trivial to implement that, and multiple changes in the code is required (mostly because json doesn't support integer keys directly...)

s-cerevisiae avatar Apr 20 '22 16:04 s-cerevisiae