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

[Bug]: separator_visible is not used from the highlights for custom separator style #924

Open
1 task done
maulik13 opened this issue Jun 13, 2024 · 4 comments
Open
1 task done
Labels
bug Something isn't working

Comments

@maulik13
Copy link

maulik13 commented Jun 13, 2024

Is there an existing issue for this?

Yes, but it was closed. #859

I understand that you do not want to provide full flexibility, however what is provided in the plugin should work consistently. The option with custom separators in separator_style is only about taking in left and right characters, the coloring of them should be the same as standard options like "slant".

I am open to creating a PR for this.

  • I have searched the existing issues

What happened?

When configuring highlights separator_visible has no effect on separators set using separator_style = {'', ''}. I did not find any reference to that parameter in the code when using individual separators.

What did you expect to happen?

Plugin should respect visible, selected for custom separators the same way it does for standard separator styles.

Config

I am using LazyVim to load the plugin.

-- Customize the entire bufferline theme from catppuccin
local function getHighlights()
  local C = require("catppuccin.palettes").get_palette("macchiato")
  local O = require("catppuccin").options
  local transparent_background = O.transparent_background
  local active_bg = transparent_background and "NONE" or C.crust
  local inactive_bg = transparent_background and "NONE" or C.base
  local separator_fg = O.transparent_background or C.crust
  local inactive_fg = C.surface2
  local styles = { "bold", "italic" }

  return {
    -- buffers
    background = { bg = inactive_bg, fg = inactive_fg },
    buffer_visible = { fg = inactive_fg, bg = inactive_bg },
    buffer_selected = { fg = C.text, bg = active_bg, bold = true, italic = true },
    -- Duplicate
    duplicate_selected = { fg = C.text, bg = active_bg, bold = true, italic = true },
    duplicate_visible = { fg = C.surface1, bg = inactive_bg, bold = true, italic = true },
    duplicate = { fg = C.surface1, bg = inactive_bg, bold = true, italic = true },
    -- tabs
    tab = { fg = C.surface2, bg = inactive_bg },
    tab_selected = { fg = C.sky, bg = C.surface1, bold = true, italic = true },
    tab_separator = { fg = separator_fg, bg = inactive_bg },
    tab_separator_selected = { fg = C.surface1, bg = C.surface1 },
    tab_close = { fg = C.red, bg = inactive_bg },

    indicator_visible = { fg = C.peach, bg = inactive_bg, bold = true, italic = true },
    indicator_selected = { fg = C.peach, bg = active_bg, bold = true, italic = true },
    -- separators
    separator = { fg = C.red, bg = inactive_bg },
    separator_visible = { fg = inactive_bg, bg = inactive_bg },
    separator_selected = { fg = inactive_bg, bg = active_bg },
    offset_separator = { fg = C.red, bg = inactive_bg },
    -- close buttons
    close_button = { fg = C.surface1, bg = inactive_bg },
    close_button_visible = { fg = C.surface1, bg = inactive_bg },
    close_button_selected = { fg = C.red, bg = active_bg },
    -- Empty fill
    fill = { bg = inactive_bg },
    -- Numbers
    numbers = { fg = C.subtext0, bg = inactive_bg },
    numbers_visible = { fg = C.subtext0, bg = inactive_bg },
    numbers_selected = { fg = C.subtext0, bg = active_bg, bold = true, italic = true },
    -- Errors
    error = { fg = inactive_fg, bg = inactive_bg },
    error_visible = { fg = inactive_fg, bg = inactive_bg },
    error_selected = { fg = C.red, bg = active_bg, bold = true, italic = true },
    error_diagnostic = { fg = inactive_fg, bg = inactive_bg },
    error_diagnostic_visible = { fg = inactive_fg, bg = inactive_bg },
    error_diagnostic_selected = { fg = C.red, bg = active_bg },
    -- Warnings
    warning = { fg = inactive_fg, bg = inactive_bg },
    warning_visible = { fg = inactive_fg, bg = inactive_bg },
    warning_selected = { fg = C.yellow, bg = active_bg, bold = true, italic = true },
    warning_diagnostic = { fg = inactive_fg, bg = inactive_bg },
    warning_diagnostic_visible = { fg = inactive_fg, bg = inactive_bg },
    warning_diagnostic_selected = { fg = C.yellow, bg = active_bg },
    -- Infos
    info = { fg = inactive_fg, bg = inactive_bg },
    info_visible = { fg = inactive_fg, bg = inactive_bg },
    info_selected = { fg = C.sky, bg = active_bg, bold = true, italic = true },
    info_diagnostic = { fg = inactive_fg, bg = inactive_bg },
    info_diagnostic_visible = { fg = inactive_fg, bg = inactive_bg },
    info_diagnostic_selected = { fg = C.sky, bg = active_bg },
    -- Hint
    hint = { fg = inactive_fg, bg = inactive_bg },
    hint_visible = { fg = inactive_fg, bg = inactive_bg },
    hint_selected = { fg = C.teal, bg = active_bg, bold = true, italic = true },
    hint_diagnostic = { fg = inactive_fg, bg = inactive_bg },
    hint_diagnostic_visible = { fg = inactive_fg, bg = inactive_bg },
    hint_diagnostic_selected = { fg = C.teal, bg = active_bg },
    -- Diagnostics
    diagnostic = { fg = inactive_fg, bg = inactive_bg },
    diagnostic_visible = { fg = inactive_fg, bg = inactive_bg },
    diagnostic_selected = { fg = C.subtext0, bg = active_bg, style = styles },
    -- Modified
    modified = { fg = C.peach, bg = inactive_bg },
    modified_visible = { fg = C.peach, bg = inactive_bg },
    modified_selected = { fg = C.peach, bg = active_bg },
  }
end

return {
  "akinsho/bufferline.nvim",
  event = "VeryLazy",
  dependencies = {
    "catppuccin/nvim",
  },
  keys = {
    { "<Tab>", "<Cmd>BufferLineCycleNext<CR>", desc = "Next tab" },
    { "<S-Tab>", "<Cmd>BufferLineCyclePrev<CR>", desc = "Prev tab" },
  },
  opts = {
    options = {
      themable = true,
      always_show_bufferline = true,
      -- show_buffer_close_icons = true,
      show_close_icon = false,
      -- numbers = function(opts)
      --   return string.format("%s", opts.raise(opts.ordinal))
      -- end,
      indicator = {
        style = "underline",
      },
      -- This does not work due to a bug, will wait or fix this later
      separator_style = { "", "" },
      -- separator_style = "slant",
      offsets = {
        {
          filetype = "neo-tree",
          text_align = "left",
          text = "Explorer",
        },
      },
    },
    highlights = getHighlights(),
  },
}

Additional Information

...

commit

Checked the main branch

@maulik13 maulik13 added the bug Something isn't working label Jun 13, 2024
@maulik13 maulik13 changed the title [Bug]: separator_visible is not used from the highlights [Bug]: separator_visible is not used from the highlights for custom separator style Jun 13, 2024
@Finii
Copy link

Finii commented Jul 11, 2024

What I see from the behavior, is that separator_style = {'A', 'B'} is not like slant, at least not what I would have expected.

slant uses two different glyphs, one always left and one always right of the tab text. When they would be A and B that would result in

A tab1 BA tab2 BA tab3 B

But in contrast the code actually generates 'thin' separators, means only one character between the tabs like

X tab1 X tab2 X tab3 X

where X is separator_style[1] when the tab is focused and separator_style[2] unfocused (or was it the other way around?).

This can be for example used to have the focused tab with an X while the other have a O like

O tab1 X tab2 O tab3 O tab4

I'm not sure this is what had been intended, and that can be changed by the following diff - meaning that the two custom characters are used one left and one right.

diff --git a/lua/bufferline/ui.lua b/lua/bufferline/ui.lua
index 440b203..8424c8f 100644
--- a/lua/bufferline/ui.lua
+++ b/lua/bufferline/ui.lua
@@ -251,7 +251,7 @@ end
 --- @param focused boolean
 --- @param style table | string
 local function get_separator(focused, style)
-  if type(style) == "table" then return focused and style[1] or style[2] end
+  if type(style) == "table" then return style[1], style[2] end
   ---@diagnostic disable-next-line: undefined-field
   local chars = sep_chars[style] or sep_chars.thin
   if is_slant(style) then return chars[1], chars[2] end

Regarding the 'wrong color', at least I see something that can count as 'wrong color'.
The color can be controlled by indicator_visible resp indicator_selected, and it is displayed even though I have indicators disabled (I though) via indicator = { style = 'none'},.

@maulik13
Copy link
Author

@Finii thank you for taking time to reply with details. I know that the author of the plugin does not intend to provide full flexibility with custom separators, so I am not expecting this to be sorted. But I would be happy to open a PR if the author is willing. Otherwise, when I get a chance I will fork and update the code fix this issue regardless.

@Finii
Copy link

Finii commented Jul 17, 2024

Now I also read the previous Issue you linked.
This triggered my curiosity.

Well, what are these custom separator_style things anyhow, lets try:

image

Ah, with separator_style = { a, b } with a and b being strings.

  • The custom separators can be multiple chars long
  • The custom separators are only printed between tabs
  • The custom separators are not printed between the last tab and a continuation/overflow marker (not shown in pic)
  • When a tab is visible (or active) string a is printed after it; string b is used otherwise

I can hardly imagine a useful setting for a and b here.
But anyhow, the issue(s) is about the coloring. Lets check.

First with normal standard separators, where I adjusted the colors for easy identification:

image

and with custom separators, they all turn red:

image

Ok, this does not look good and seems like a bug.

But what was the intention for the custom separator style. There is a comment in the get_separator() function, and it seems like this is the expected use-case (all colors default):

image

And now all the behavior makes sense.

  • It does not make sense to use different colors based on active or visible, because the separator is potentially/always used for different types of tabs
  • It does make sense to not show the custom separators on the far left and right
  • It makes sense to just depend the rhs of the tab for the selected separator

Well, at least for the active and the invisible tabs. The visible-but-not-active (number 4 in the image), this still looks strange.

So I must admit, this is not a bug but the behavior is needed to support this one specific use-case that seemed to have driven the development of the custom separators. It's just not what I (and you and other) would have expected this feature to be for. :-D


Edit:

The code is really really old, just dove into git blame.
It seems that people wanted to adjust how wide the left and right vertical bar is, some wanted that smaller, some bigger, and this allowed to have any combination/selection of thick or thin or middle or whatever you like, from the box drawing glyphs.

image

Example archeological blame, see for example 1a1545b and cab12e4

@maulik13
Copy link
Author

@Finii that was a great analysis and you concluded very similar to what I had in mind. Technically things are working as intended if we go by the code and its comments. But based on how configuration is provided to a user, results are unexpected. I suppose this can be considered a limitation as intended by the author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants