search

f4z3r / gruvbox-material.nvim

Material Gruvbox colorscheme for Neovim written in Lua
MIT License
32 stars 4 forks source link

fix: update for new treesitter capture names #4

Closed f4z3r closed 4 months ago

f4z3r commented 4 months ago

This should fix any issues due to new capture names in nvim-treesitter. I updated the highlight links in a way that should not modify the highlighting. The only sections where I am a bit unsure is the markdown stuff. Considering it was broken and I don't have a color reference for stuff like quotes or code blocks from before it broke (have been using a slightly modified version of this for quite a time), I used what seemed reasonable.

I will test it against some code samples manually. Not sure how one could automate regression tests for colorschemes ...

Fixes #2

logan-connolly commented 4 months ago

Great work @f4z3r! I just tried out the fix/treesitter branch locally and I am indeed now getting highlights in markdown files :)

The only sections where I am a bit unsure is the markdown stuff. Considering it was broken and I don't have a color reference for stuff like quotes or code blocks from before it broke (have been using a slightly modified version of this for quite a time), I used what seemed reasonable.

You are right. It does appear that there is a regression in the markdown code blocks. Oddly enough, the code blocks were the only place in the markdown file where I was getting highlights. It appears that it was using highlights from the underlying programming language treesitter implementation.

You can see the difference in the screenshot (left=fix/treesitter, right=master):

image

I am planning to have a closer look on the weekend. But judging by the green colors of the function and methods, I imagine it is taking the values defined here (L482 highlights.lua):

    -- functions
    ["@function"] = { link = "Green" },
    ["@function.builtin"] = { link = "@function" },
    ["@function.call"] = { link = "@function" },
    ["@function.macro"] = { link = "@function" },

    ["@function.method"] = { link = "Green" },
    ["@function.method.call"] = { link = "@method" },

    ["@constructor"] = { link = "Green" },
    ["@operator"] = { link = "Orange" },

I will test it against some code samples manually. Not sure how one could automate regression tests for colorschemes ...

This is a good question, and I am not sure if it is possible or if it is viable to test colorschemes. I will take a look at some popular lua colorscheme projects; maybe there is some inspiration to be had there.

f4z3r commented 4 months ago

Oh, damn, that is quite a difference actually 😆

Yes, tree-sitter uses sub-parsers for the code blocks with the respective languages. Since there most parsers actually apply both the old and the new capture groups, it is normal that you still had the highlighting. But using Markdown with various code blocks in various languages could be a really nice option to have side-by-side comparisons. I will create such a simple in the repo such that we can compare easily the output, with as many capture groups as possible in short snippets 👍🏽

f4z3r commented 4 months ago

Ok, it already looks much better now:

image

I will extend the sample a bit to ensure that I really cover everything. There are some minor changes that I added as notes to the changelog. What do you think? IMO they make sense to introduce, even though they are not 100% backward compatible.

f4z3r commented 4 months ago

Ok @logan-connolly, I would say it looks good:

image

There are a few tiny changes:

logan-connolly commented 4 months ago

I don't think it looks good; I think it looks really great! Really awesome work, man.

IMO they make sense to introduce, even though they are not 100% backward compatible.

I want to backtrack what I said in the original issue in the archived repository regarding backwards compatibility. You should totally go for it if you see clear improvements. The changes that you made are good improvements to the library IMO. There is always the option to use the one from WittyJudge (or to fork).

Unfortunately, I did not get as much time as I'd hope to look deeper into this, but I've pulled in the latest changes and the output in the sample file looks great. So, I have no change requests. I'd say let it rip!

PS: I particularly like the diff improvement :)