search

nordtheme / vim

An arctic, north-bluish clean and elegant Vim theme.
https://www.nordtheme.com/ports/vim
MIT License
2.52k stars 275 forks source link

Aggressive error highlight with Treesitter #269

Closed georg3tom closed 3 years ago

georg3tom commented 3 years ago

Significant parts of the code bg gets highlighted in red because of syntax errors.

2021Jul23::015333

Occurs only when treesitter highlight is enabled NVIM v0.5.0 with latest nvim-treesitter

tgturner commented 3 years ago

I am also experiencing this issue when using Treesitter with nvim.

It looks like the TSError highlighting group should be removed. You can see how the Dracula theme handled it here. I would submit a PR but I'm not currently at a computer I can commit from.

georg3tom commented 3 years ago

I'll open a PR

arcticicestudio commented 3 years ago

Hi @georg3tom and @tgturner :wave:, thanks for your contributions :+1:

Based on the references it looks like removing the group is currently the only solution to get rid of this, but I wonder how errors are then highlighted (if at all)? Will Neovim's LSP take over to only highlight the matched elements with the correct style (i.e. only change the foreground color of a single word) or will errors will appear the same like any other element and only messages from linters (maybe with arrow marker) will indicate that there is an error at all?

I have no problem removing the group, but I would just like to understand how this affects the user experience because I don't Neovim for active development (yet) and don't have a running setup yet to test this change.

tgturner commented 3 years ago

Hey @arcticicestudio this change does not impact Neovim’s LSP error highlighting (which still works correctly) but only impacts Treesitter (Neovim’s improved syntax highlighting) error highlights.

since Treesitter is constantly parsing the file, when typing you are pretty much garunteed to have invalid syntax the Treesitter can’t parse at points. It looks like most themes are not relying on Treesitter’s error groups because of this issue, but offloading the work to linters and LSP client to report syntax errors. I’m not currently at my computer but would be happy to paste an image of what LSP error highlighting looks like after the proposed change.

arcticicestudio commented 3 years ago

Thanks @tgturner, when LSP takes care of it we can remove the group without having to worry. I'll merge #270 to get this into the next release version 0.18.0 with ETA for next Sunday (2021-09-12).