zbirenbaum / neodim

Neovim plugin for dimming the highlights of unused functions, variables, parameters, and more
315 stars 10 forks source link

[Bug] Neodim uses outdated highlight groups based on faulty heuristic #22

Closed akinsho closed 1 year ago

akinsho commented 1 year ago

Hi @zbirenbaum ๐Ÿ‘‹๐Ÿฟ

So I've noticed that this plugin still relies on setting TS<Capture>Unused highlights in my case and dug around a bit and found that the following line is the reason why.

https://github.com/zbirenbaum/neodim/blob/c346344ade2ce709e6bd282f10e43778672b861e/lua/neodim/util.lua#L17

It essentially uses the existence of an outdated highlight group as a check if it should use old or new style highlight groups. The issue with this is that if the colorscheme author is keeping these highlights around for backwards compatibility but the focus has moved to @<Capture> highlights then rather than using those values the plugin uses the older ones.

I think a better check would be something like

local to_ts_hl_group = function (str)
  local is_updated, _ = pcall(vim.api.nvim_get_hl_by_name, "@warning", true);
  if not is_updated then
    return nil
  end
  return "@"..str
end

Rather than checking if the old highlight group exists it'd be better to check if the new one exists since it used to be an error to have highlights with that sort of name in neovim so it would error for anyone on an old version and would enter the nil case and use the old style highlights

zbirenbaum commented 1 year ago

I think this is correct. Since you already provided a likely solution, you can PR this If you'd like credit, and I'll do some quick tests then merge it. If you don't care about that type of thing I'm happy to add it in myself, just let me know.

akinsho commented 1 year ago

I don't really mind much tbh, I do have the changes locally already though so happy to push a PR