ishan9299 / nvim-solarized-lua

solarized colorscheme in lua for nvim 0.5
MIT License
184 stars 54 forks source link

Map `Delimiter` to `Normal` instead of `Special` #39

Closed luator closed 2 years ago

luator commented 2 years ago

As mentioned in #33 the Delimiter highlight group is currently mapped to Special, resulting in dots, parentheses, etc. to be orange. This PR maps it to Normal instead, so they don't stand out so much. Here is an before/after screenshot:

solarized

I see that this is a matter of taste, though, so I if you don't agree with the change, I'm completely fine with it!

How I tested: I had this change applied locally for a while now and at least for the Python and C++ code I'm working with, it looks good to me. I normally only use the normal "solarized" scheme, though, so I quickly checked the -flat/-high/-low variants and the seemed good as well.

Resolves #33

ishan9299 commented 2 years ago

Looking here the vim-solarized8 theme does the same https://github.com/lifepillar/vim-solarized8/blob/28b81a4263054f9584a98f94cca3e42815d44725/colors/solarized8.vim#L25.

I'd say the issue you are facing if still bothers you when using vim-solarized8 then I would be merge it by making it an option.

luator commented 2 years ago

Oh, I see. Makes sense to keep it consistent then.

Instead of adding an option I thought I could just add highlight! link Delimiter Normal in my config to achieve the same (without the need of adding more complexity to the colorscheme itself). It wasn't working, though, because just changing Delimiter didn't have an effect on TSPunctBracket, TSPunctDelimiter, etc. I finally solved it by replacing all the syntax['foo'] = syntax['bar'] assignments with highlight link commands. This way, overwriting one group also affects all the groups linking to it.

Would you be open to a PR applying this change? I think it would be nice as it would make customisation of the colorscheme easier but I think it is to some degree a breaking change that might mess up existing configurations.

chrs8 commented 2 years ago

As the mapping of Delimiter to Special is in vim-solarized8 it maybe should be left that way. The "problem" only occurs when treesitter syntax highlighting is turned on (and vim-solarized8 has no TS support). So TSPunctBracket and TSPunctDelimiter should not be mapped to Delimiter. The vim naming convention says Delimiter is a "character that needs attention".

@luator If you want to override the colorscheme (treesitter highlights), you can use the custom_captures option of nvim-treesitter. My configuration:

require('nvim-treesitter.configs').setup {
  highlight = {
    enable = true,
    custom_captures = {
        ["punctuation.bracket"] = "Variable",
        ["punctuation.delimiter"] = "Variable",
        ["keyword.function"] = "Keyword",
        ["include"] = "Keyword",
    }
  },
luator commented 2 years ago

Indeed, when I disable treesitter highlighting, the parentheses, commas, etc. don't have any special highlighting. It is not really clear to me, what the Delimiter group is supposed to be used for (which characters do need "special attention"?) but I understand that it may not be meant for normal punctuation stuff. In any case, I think this PR (changing Delimiter) should be closed. I now fixed it for myself locally by linking TSPunctBracket and TSPunctDelimiter to Normal in my config (similar to chrs8's suggestion).

Independent of this, I still think it might be nice to use linking instead of copying values to make these kind of customisation easier. @ishan9299 let me know in case a PR for this would be welcome.