HiPhish / rainbow-delimiters.nvim

Rainbow delimiters for Neovim with Tree-sitter
https://gitlab.com/HiPhish/rainbow-delimiters.nvim
Apache License 2.0
532 stars 39 forks source link

fix: Improve macro highlighting in c and cpp #80

Closed Danielkonge closed 2 months ago

Danielkonge commented 9 months ago

This improves the highlighting delimiters inside c/c++ macros.

Note that this won't fix all the macro highlighting problems, since we clear namespace by row without column information. If we can limit lib.clear_namespace to the correct columns, then this should fix all the macro highlighting problems.

It's important that we keep the parent_lang ~= lang check if we want to continue having good highlighting in rust macros, and since I haven't noticed problems outside of c/c++, I guess it usually works fine with the current code.

HiPhish commented 9 months ago

Can this issue not be fixed upstream in Neovim or the parser? Adding a language-specific hack feels really nasty. If it cannot we should at least have a table of these languages.

local self_injecting_languages = {c = true, cpp = true}

-- ...
elseif (parent_lang ~= lang or self_injecting_languages[parent_lang]) and changes[1] then
-- ...
Danielkonge commented 9 months ago

Can this issue not be fixed upstream in Neovim or the parser? Adding a language-specific hack feels really nasty. If it cannot we should at least have a table of these languages.

local self_injecting_languages = {c = true, cpp = true}

-- ...
elseif (parent_lang ~= lang or self_injecting_languages[parent_lang]) and changes[1] then
-- ...

I also don't really like this hack, but I did try to make an upstream issue ( https://github.com/neovim/neovim/issues/26837 ) that was closed quickly, though I might not have explained clearly what the problem was. I think the upstream changes are supposed to make the injections more like an injection of another language (which makes sense with our check for parent_lang ~= lang), and I am not sure they want to change it for c or c++. Maybe it is really rust that should change to be more like c, so we could remove the parent_lang ~= lang test? I am not sure if there is a consensus for which way is preferred?

The problem is not really self injecting languages per se, it is more the difference between the parsers of the self injecting languages.

It might make sense to try to make an upstream issue that better explains what we would want.

Also, as a side note, I made an upstream PR ( https://github.com/neovim/neovim/pull/26842 ) to allow better control of the columns for nvim_buf_clear_namespace, which seems to allow correct highlighting together with this fix + correct input of columns.

HiPhish commented 9 months ago

I am trying to understand what is going on here.

The issue is we are inside an injected language, such as Lua code inside a Mardown document; in that case the check parent_lang ~= lang applies and we assign changes = {{tree:root():range()}} to highlight the entire Lua block. On the other hand, if the injected language is the same as the parent language we do nothing because the parent tree has already or will be highlighted and will include the current child.

However, this last behaviour has changed, now changes does not include any injected C or C++ nodes, even if the parent language is the same. For some odd reason Rust still have the old behaviour though.

Is my understanding correct?

Danielkonge commented 9 months ago

I am trying to understand what is going on here.

The issue is we are inside an injected language, such as Lua code inside a Mardown document; in that case the check parent_lang ~= lang applies and we assign changes = {{tree:root():range()}} to highlight the entire Lua block. On the other hand, if the injected language is the same as the parent language we do nothing because the parent tree has already or will be highlighted and will include the current child.

However, this last behaviour has changed, now changes does not include any injected C or C++ nodes, even if the parent language is the same. For some odd reason Rust still have the old behaviour though.

Is my understanding correct?

Yes, from my understanding that is correct. Rust still captures the injected macros in the parent parser, but C/C++ doesn't anymore from what I can tell. I guess the new C/C++ behavior might make more sense (since it is similar to injections of other languages), but it is annoying that the behavior is different (from what I can tell).

Danielkonge commented 2 months ago

I will close this too. If we want to be more careful with injections, we should do that when rewriting the global strategy in #125.