romgrk / barbar.nvim

The neovim tabline plugin.
2.2k stars 81 forks source link

feat: pinned buffers stay visible #408

Closed Iron-E closed 1 year ago

Iron-E commented 1 year ago

Pinned buffers now always stay visible in the bufferline. Additionally, they take up less space— they use the minimum padding, and no longer show a filename or pin icon by default.

To restore the old pinned buffer look, do:

require'barbar'.setup {
  icons = {
    pinned = {
      button = '',
      filename = true,
      separator = {right = ''},
    },
  },
}

Todo:


Closes #393. Closes #401.

otavioschwanck commented 1 year ago

Loved the idea!

Iron-E commented 1 year ago

Initial implementation is done. No configuration is required (unless you want to customize the look of pinned buffers).

Edit: oops found a bug

Edit 2: Bug squashed. Feel free to try @otavioschwanck

(I'll walk through the reasons for my implementation choices later, since I'm experimenting with alternate implementations for more perf)

otavioschwanck commented 1 year ago

@Iron-E Nice work!

Just a bug:

For some reason, the separators of non pinned buffers get messed:

image
Iron-E commented 1 year ago

Thanks for testing! Config?

otavioschwanck commented 1 year ago

Thanks for testing! Config?

      require('bufferline').setup({
        icons = {
          -- Configure the base icons on the bufferline.
          buffer_index = false,
          buffer_number = false,
          button = '',
          -- Enables / disables diagnostic symbols
          diagnostics = {
            [vim.diagnostic.severity.ERROR] = {enabled = true, icon = 'ff'},
            [vim.diagnostic.severity.WARN] = {enabled = false},
            [vim.diagnostic.severity.INFO] = {enabled = false},
            [vim.diagnostic.severity.HINT] = {enabled = false},
          },
          filetype = {
            -- Sets the icon's highlight group.
            -- If false, will use nvim-web-devicons colors
            custom_colors = false,

            -- Requires `nvim-web-devicons` if `true`
            enabled = true,
          },
          separator = {left = '▎', right = ''},

          -- Configure the icons on the bufferline when modified or pinned.
          -- Supports all the base icon options.
          modified = {button = '●'},
          pinned = {buffer_index = true, filename = true},

          -- Configure the icons on the bufferline based on the visibility of a buffer.
          -- Supports all the base icon options, plus `modified` and `pinned`.
          alternate = {filetype = {enabled = false}},
          current = {buffer_index = false},
          inactive = {button = ''},
          visible = {modified = {buffer_number = false}},
        },
        -- icon_pinned = '󰐃',
        exclude_ft = {'netrw'},
        -- closable = false,
        minimum_padding = 1,
        maximum_padding = 1
      })
Iron-E commented 1 year ago

@Iron-E Nice work!

Just a bug:

For some reason, the separators of non pinned buffers get messed: image

I can reproduce the issue with your config.

otavioschwanck commented 1 year ago

just a quick request: Is possible to create an highlight for the pinned buffers? Pinned and PinnedActive

Iron-E commented 1 year ago

just a quick request: Is possible to create an highlight for the pinned buffers? Pinned and PinnedActive

That's probably in scope of #199. We can certainly do that, probably wouldn't be too hard

otavioschwanck commented 1 year ago

Something else:

The padding of the pinned buffers are different from the non pinned:

Pinned:

image

Non-pinned:

image
Iron-E commented 1 year ago

Merge the config from the original post with yours. There is a separator = {right = ' '} by default with pinned buffers now, to make it look better with the defaults. You can set pinned.separator.right to '' to restore the old functionality.

Iron-E commented 1 year ago

#416 broke this. Have to fix

Fixed by #420

I'll review the changes after rebasing on #418, when that merges to master

otavioschwanck commented 1 year ago

Tested the latest version of this branch, the pinned are not always visible now

Iron-E commented 1 year ago

Tested the latest version of this branch, the pinned are not always visible now

What commit hash? It works for me on dcf79233d42002716a9fe4548866e9f9120b2826

Iron-E commented 1 year ago

I rebased on master, but I want to double check everything and clarify with @otavioschwanck before merging

otavioschwanck commented 1 year ago

I rebased on master, but I want to double check everything and clarify with @otavioschwanck before merging

the pin visibility is working but reordering with BufferlineMoveNext and BufferlineMovePrev causes the barbar to blink:

gif

romgrk commented 1 year ago

@Iron-E ping me for a review before merging, after you fix the bug above.

Iron-E commented 1 year ago

the pin visibility is working but reordering with BufferlineMoveNext and BufferlineMovePrev causes the barbar to blink

I can reproduce, only with animations on. Thanks for the report :)

Edit: seems to be a problem in the render.lua file.

Edit 2: The issue seems to be because of Layout.calculate_buffers_position_by_buffer_number(). It doesn't take into account the fact that pinned buffers are a static segment yet.

Iron-E commented 1 year ago

Alright, animations should be working again. @otavioschwanck let me know if you encounter more issues— your testing has been a big help so far!


@romgrk FYI, since this affects the animation code: pinned buffers' position / calculated_position is on a separate track now. E.g.:

| a | b | c   x | d    x |
  0   1 | 0       1

This is because the pinned buffers are technically a different segment, similar to an offset. However, that segment has groups in it, unlike an offset, so we still need to track the positions within that segment.

A couple things I want to try before final review. I'll let you know.

Iron-E commented 1 year ago

Final note for the review: I gave pinned buffers a right '🮇' separator, because of scenarios like this:

otavioschwanck commented 1 year ago

@Iron-E now is working.

image

Just change the default separator of the pinned, the current is just a block for me (i have nerdfont installed)

Iron-E commented 1 year ago

@otavioschwanck What font are you using? That's just a regular unicode character

I guess an alternative could be this though it's half the width. What do you think @romgrk? Is this a font issue, or do we choose a different symbol?

romgrk commented 1 year ago

Isn't that character just the right-aligned version of our default separator? If it's a standard unicode character I'm ok with it. We can adjust if we get more bug reports.

@otavioschwanck How does this character display in your terminal? '▎'

Iron-E commented 1 year ago

Isn't that character just the right-aligned version of our default separator?

Yup. "Right One Quarter Block" vs "Left One Quarter Block"

romgrk commented 1 year ago

For some reason the original block elements characters didn't have the right one-quarter block, so instead of it being in the basic multilingual plane it's in the supplementary multilingual plane. Even in my browser I see the left one correctly, but the right one doesn't display. This is annoying. But I think I'd still go with that character, it's the correct one.

Iron-E commented 1 year ago

For some reason the original block elements characters didn't have the right one-quarter block, so instead of it being in the basic multilingual plane it's in the supplementary multilingual plane.

Ahh. That is annoying.

romgrk commented 1 year ago

I'm not sure about having the separator in the right instead of the left, it feels a bit off to me:

image image

Could we keep the left separator as the icon, and add the right separator just for the final item in the pinned buffers? Or can we be smart and add it only if the unpinned section doesn't start with a separator? Or can we make the unpinned section always start with a separator? edit: I think that last option is my favourite, keeps things neat.

And does that work with your configuration with these? image

otavioschwanck commented 1 year ago

'▎'

image
romgrk commented 1 year ago

The move animation is a bit off, you can go frame-by-frame through this video to see.

https://user-images.githubusercontent.com/1423607/230149606-46fc4559-897e-439e-bf72-c88413701e2b.mp4

I wanted to have pin/unpin animations as well, not sure if you wanna do them or if you prefer to leave them to me, I can do in a separate PR.

Iron-E commented 1 year ago

Or can we be smart and add it only if the unpinned section doesn't start with a separator? Or can we make the unpinned section always start with a separator?

I was looking at doing something like this for #66. It's possible but it would complicate this:

  1. store the separator in each group_clump
  2. check that both:
    1. the last group of the pinned tabs is not a right separator,
    2. the first group of the non-pinned tabs is not a left separator
  3. if the above is true, insert the left separator for the activity of the first non-pinned buffer

This also requires doing parallel calculations in layout, which is hard since layout doesn't know about the scroll, or where groups or currently sliced.

It seemed simpler just to say "pinned buffers have a right separator"

Iron-E commented 1 year ago

I wanted to have pin/unpin animations as well, not sure if you wanna do them or if you prefer to leave them to me, I can do in a separate PR.

That's probably a task for you 😅

The animation code is dark magic to me.

romgrk commented 1 year ago

A different solution to the separator problem could also be to add the arrow icons if there is some scroll on. Cuz it's only in that case that the right separator becomes necessary, right? Wdyt?

If needed we can always lift scroll calculations inside layout, it's not a bad place to do these.

Iron-E commented 1 year ago

A different solution to the separator problem could also be to add the arrow icons if there is some scroll on. Cuz it's only in that case that the right separator becomes necessary, right? Wdyt?

68 has been a long-standing issue, and I have intended to work on that eventually. When we do that it would alleviate the need for right separators (as a default for pinned buffers) entirely.

Were you thinking we wrap #68 into this PR, use right separators until #68 is implemented, or something else?

romgrk commented 1 year ago

Let's not add right separators in this PR if we're not going with those. Up to you if you want to do #68 here or we do in a different PR, I'm fine with not having any of those temporarily.

Iron-E commented 1 year ago

I think it would be better to do in another PR. I'll remove the right separator for now then.

As for the animations, I amended the previous commit with some changes that made things look smoother. Let me know if it's still broken!

romgrk commented 1 year ago

You can't reproduce locally? This is a frame of what I see at some point during the animation. image

https://user-images.githubusercontent.com/1423607/230178340-6c173327-d77f-4ff2-8f9a-6b88fc9f7b26.mp4

It's also broken for unpinned buffers: image

https://user-images.githubusercontent.com/1423607/230178067-50055716-2d88-4ee7-acf6-f266fb44b311.mp4

If you have trouble seeing slow down the MOVE_DURATION:

https://user-images.githubusercontent.com/1423607/230179071-0588499d-b6d9-4cd3-88cf-e389f4315463.mp4

You can also use the debug code I added in render to debug.

romgrk commented 1 year ago

The active buffer should always be on top.

romgrk commented 1 year ago

Nevermind, master is broken as well. Just merge it, I'll deal with animations.

Iron-E commented 1 year ago

I always had a bad eye for things like this; I checked master but didn't see anything either but didn't really trust myself. Thanks for double checking :)

romgrk commented 1 year ago

In case of doubt, set all the animation durations to 2000, easy to debug then.

Iron-E commented 1 year ago

In case of doubt, set all the animation durations to 2000, easy to debug then.

That's a great tip— thank you!