utilyre / barbecue.nvim

Visual Studio Code inspired breadcrumbs plugin for the Neovim editor
MIT License
776 stars 31 forks source link

Switching themes will cause an error #44

Closed ofseed closed 1 year ago

ofseed commented 1 year ago

As the title says, after neovim starts, if you change the current theme, an error will be reported, and the color of barbecue will disappear.

before: image

after changing the theme (even changing to the same theme): image

Error message:

Error executing vim.schedule lua callback: ...s/code/nvim/barbecue.nvim/lua/barbecue/ui/components.lua:26: invalid key: 
stack traceback:
    [C]: in function 'nvim_set_hl'
    ...s/code/nvim/barbecue.nvim/lua/barbecue/ui/components.lua:26: in function 'get_file_icon'
    ...s/code/nvim/barbecue.nvim/lua/barbecue/ui/components.lua:110: in function 'get_basename'
    ...ed/Documents/code/nvim/barbecue.nvim/lua/barbecue/ui.lua:62: in function 'create_entries'
    ...ed/Documents/code/nvim/barbecue.nvim/lua/barbecue/ui.lua:146: in function <...ed/Documents/code/nvim/barbecue.nvim/lua/barbecue/ui.lua:135>

Note the error only generates when opening a new type of file.

ofseed commented 1 year ago

By the way, the auto theme seems to be useless. Even I am using catppuccin as my colorscheme, The color will be the default unless I set the theme to catppuccin explicitly.

utilyre commented 1 year ago

Hello. Checkout feature/theme-generation, it should be working. (both "auto" and changing colorscheme)

ofseed commented 1 year ago

On this branch, there must be errors generated at the moment of switching:

Error executing lua callback: ...cal/share/nvim/lazy/barbecue.nvim/lua/barbecue/theme.lua:185: invalid key: 
stack traceback:
    [C]: in function 'nvim_set_hl'
    ...cal/share/nvim/lazy/barbecue.nvim/lua/barbecue/theme.lua:185: in function 'load'
    ...ed/.local/share/nvim/lazy/barbecue.nvim/lua/barbecue.lua:56: in function <...ed/.local/share/nvim/lazy/barbecue.nvim/lua/barbecue.lua:55>

Colors other than file icons seem to work fine though: image

nyngwang commented 1 year ago

@utilyre Hi, sir. What the OP has reported is exactly the same thing I experienced during my debugging of #40. I highly recommend you write some tests investigating the linked lines at the end of this comment, especially the for-loop(lines 71 to 78). And these are some of my conclusions, which hopefully can save you some time:

  1. I'm pretty sure that this problem is related to the start-up sequence lazy.nvim designed to run those config = .... If you log the two loop variables, i.e. key, name, at line 71, you will see those values printed before NeoVim started. (You will need to pause the video frame by frame to see the result of prints)

https://user-images.githubusercontent.com/24765272/210172569-0ce0b958-2876-4a9e-a3d9-474fd1581f45.mov

  1. But then, lazy.nvim seems to discard these values thereafter. So, in fact, at line 75 the API vim.tbl_extend will receive a theme[nil] as the second parameter, which causes the error message invalid key:

    [...] invalid key: 
    stack traceback:
        [C]: in function 'nvim_set_hl'
    [...]
  2. All your current commits didn't fix the coloring issue mentioned in #40 as far as I know. I still get everything cleared from :hi(I tested it again when writing this.)

  3. Based on the error message provided by @ofseed above, he/she is using lazy.nvim. So my conclusions apply.

    [C]: in function 'nvim_set_hl' ...cal/share/nvim/lazy/barbecue.nvim/lua/barbecue/theme.lua:185: in function 'load'

https://github.com/utilyre/barbecue.nvim/blob/95f145a9445df954918e3751dd51ba2446606a31/lua/barbecue/theme.lua#L61-L78

Feel free to tag me once you solve it since I'm also interested in the reason why those entries of imported tables will eventually become nil by some magic of lazy.nvim.

utilyre commented 1 year ago

I don't think it is the case here, but i guess it's worth mentioning that barbecue.setup should be called once the colorscheme and nvim-cmp (because some colorschemes, like tokyonight, do not define all of the cmp highlights which are being used as fallbacks) are loaded for it to function correctly.

I'm planning to make the auto option only rely on the builtin neovim highlight groups though.

utilyre commented 1 year ago

Btw @nyngwang are you on nvim nightly? If so try out nvim stable and see if the bug still persists.

However, with the minimal config you gave me and the latest commit of main, this invalid key: issue is fixed (at least on my machine).

nyngwang commented 1 year ago

Btw @nyngwang are you on nvim nightly? If so try out nvim stable and see if the bug still persists.

Yes. But I just rollbacked to neovim/neovim@99cf111 since upstream is doing some breaking changes on TUI and it breaks my noice.nvim now. OK, I will try it again at this commit and then the stable one.

[...] I'm planning to make the auto option only rely on the builtin neovim highlight groups though.

Please do! I believe this is the easiest way to solve all of these troubles!

I don't think it is the case here, but i guess it's worth mentioning that barbecue.setup should be called once [...]

So in your current setup those highlight groups from :hi are not cleared?

utilyre commented 1 year ago

So in your current setup those highlight groups from :hi are not cleared?

No, they get created based on the loaded theme. I might be able to spot the issue if you give me your dotfiles.

utilyre commented 1 year ago

Made some changes again for relying on the builtin highlights.

I'm not quite sure which version of nvim brought these @enum-like hls, if somebody knows please tell me so that I can bump the required version of neovim in readme.

However, I tested them and they seem to be defined with nvim out of the box.

nyngwang commented 1 year ago

Got error if I set the autocmd myself. Here is my config:

use {
  'utilyre/barbecue.nvim', enabled = true,
  dependencies = {
    'neovim/nvim-lspconfig',
    'smiteshp/nvim-navic',
    'kyazdani42/nvim-web-devicons',
  },
  config = function ()
    require('barbecue').setup {
      attach_navic = false, -- attach navic to LSPs automatically.
      create_autocmd = false,
    }
    vim.api.nvim_create_autocmd({ 'CursorHold' }, {
      callback = function()
        require('barbecue.ui').update()
      end,
    })
    -- If I use the following autocmd then I will get an error.
    vim.api.nvim_create_autocmd({ 'ColorScheme' }, {
      callback = function()
        require('data.barbecue').set_background('Normal', 'guibg')
        -- which basically contains many lines like this:
        -- vim.cmd('hi barbecue_normal guibg=#393939')
      end,
    })
  end
}
   Error  22:16:44 msg_show.lua_error Error executing vim.schedule lua callback: ...e/nvim/lazy/barbecue.nvim/lua/barbecue/ui/components.lua:27: invalid key: 
stack traceback:
    [C]: in function 'nvim_set_hl'
    ...e/nvim/lazy/barbecue.nvim/lua/barbecue/ui/components.lua:27: in function 'get_file_icon'
    ...e/nvim/lazy/barbecue.nvim/lua/barbecue/ui/components.lua:111: in function 'get_basename'
    ....local/share/nvim/lazy/barbecue.nvim/lua/barbecue/ui.lua:62: in function 'create_entries'
    ....local/share/nvim/lazy/barbecue.nvim/lua/barbecue/ui.lua:149: in function <....local/share/nvim/lazy/barbecue.nvim/lua/barbecue/ui.lua:138>
utilyre commented 1 year ago

You should be using the theme option in config (see Configuration).

In case you want to override it by hand, you should go to Template and copy M variable's value and paste it as theme in your config. Then start changing the colors. (removing some elements, say context_file, is okay since it falls back to the auto detected/generated theme)

utilyre commented 1 year ago

Sorry, I forgot to say the feature/theme-generation branch had been merged yesterday.

nyngwang commented 1 year ago

In case you want to override it by hand, you should go to Template and copy M variable's value and paste it as theme in your config. [...]

@utilyre But I still prefer the old way of using autocmd on ColorScheme to dynamically change the guibg of all barbecue_*. With theme in config it is just a fixed value.

UPDATE: I just confirmed that the requirement can be achieved after the updates of lazy.nvim minutes ago. But it's still possible to get the following error when I change between colorschemes:

   Error  06:17:40 msg_show.lua_error E5108: Error executing lua: /Users/Foo/.config/nvim/init.lua:83: Vim(append):Error executing lua callback: ...cal/share/nvim/lazy/barbecue.nvim/lua/barbecue/theme.lua:77: invalid key: 
stack traceback:
    [C]: in function 'nvim_set_hl'
    ...cal/share/nvim/lazy/barbecue.nvim/lua/barbecue/theme.lua:77: in function 'load'
    ...l/share/nvim/lazy/barbecue.nvim/lua/barbecue/autocmd.lua:63: in function <...l/share/nvim/lazy/barbecue.nvim/lua/barbecue/autocmd.lua:62>
    [C]: in function 'cmd'
    /Users/foo/.config/nvim/init.lua:83: in function </Users/Foo/.config/nvim/init.lua:83>
stack traceback:
    [C]: in function 'cmd'
    /Users/Foo/.config/nvim/init.lua:83: in function </Users/Foo/.config/nvim/init.lua:83>
utilyre commented 1 year ago

@utilyre But I still prefer the old way of using autocmd on ColorScheme to dynamically change the guibg of all barbecue_*. With theme in config it is just a fixed value.

Honestly, it's not possible to satisfy everyone. As people wanted in #30, I made this mechanism of defining highlight groups dynamically which is inspired by bufferline.nvim and lualine.nvim. So I think going back to the way it was before is regression.

But it's still possible to get the following error when I change between colorschemes

I cannot reproduce this, make sure to use nvim stable and also change your config to use the theme option in .setup() and see if it is still giving you the error.

nyngwang commented 1 year ago

[...] So I think going back to the way it was before is regression.

Well, even after removing all my codes(so no "old way" anymore) for "using autocmd on ColorScheme to dynamically change the guibg", I still have the same error when changing between installed colorschemes: (As you can see, the highlight group fallback to use the default one pick by setting 'auto' after the same error happened many times)

https://user-images.githubusercontent.com/24765272/210536242-c9c4bcd6-12da-497d-9d02-d2950d0e6880.mov

(btw, I personally regard bufferline.nvim as a counter-pattern of Vim, since it always shows all the buffers regardless of your current tabpage.)

I cannot reproduce this, [...]

Could you try it again with lazy.nvim?(If you need more context I can provide) I believe the trouble will still exist even if I use nvim-0.8. The problem is on the API nvim_set_hl, which if it's buggy on 0.9 it's not possible to fix it by downgrading to 0.8. Will try 0.8 now and report.

update: As I expected, downgrading doesn't solve it: (The first img should convince you that I indeed downgraded it)


to reproduce you can use this config for lazy.nvim:

{
  'utilyre/barbecue.nvim', enabled = true,
  dependencies = {
    'neovim/nvim-lspconfig',
    'SmiteshP/nvim-navic',
    'nvim-tree/nvim-web-devicons',
  },
  config = function ()
    require('barbecue').setup {
      attach_navic = false,
      create_autocmd = false,
      theme = 'auto',
    }
    vim.api.nvim_create_autocmd({ 'CursorHold' }, {
      group = 'nvim_ide.lua',
      callback = function()
        require('barbecue.ui').update()
      end,
    })
  end
}
utilyre commented 1 year ago

Should be fixed now. (hotfix/colorscheme-switch branch)

nyngwang commented 1 year ago

@utilyre Hi, I just tested the branch hotfix/colorscheme-switch, and these are my observations:

And I also tested the latest commit fa83977, and these are my observations:

Btw, these are some updates from lazy.nvim which might help you debug:

nyngwang commented 1 year ago

I just checked the plugin nvim-treesitter-context and the can work with the traditional hi ... on ColorScheme without error. There are only 3 lines for setting up those highlight groups that can change the background color and it works well with lazy.nvim. Maybe it would help you debug/improve/refactor/whatsoever that is helpful for making your plugin better.

I have no more energy debugging this. Sorry for maybe being annoying. Leaving...

utilyre commented 1 year ago

@nyngwang thanks for being helpful along the way.

I've also been experiencing the default.lua theme not being able to extract some colors from the colorscheme. So I think it's more likely a weird loading order problem with lazy.nvim.

@ofseed please consider closing issue if the mentioned branch is working fine. It'll be merged into main once #186 is merged.

utilyre commented 1 year ago

It's been two weeks, so I'll close the issue myself. Feel free to re-open if this happens again.

ofseed commented 1 year ago

Sorry for missing the previous notification, thank you so much for your work. I have also been using barbecue since this branch was merged into master two weeks ago and it has worked fine.