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

Some inconsistencies in Treesitter support #268

Open joeveiga opened 3 years ago

joeveiga commented 3 years ago

Hello @arcticicestudio 👋 ! First of all, great theme! I use it everywhere lol. I was looking forward to the treesitter support for neovim and enabled it right away when it was released over the weekend. I did notice some weird behavior with typescript. In the gif below you can see the file with treesitter highlight enabled on the left side.

Kapture 2021-07-12 at 10 08 50

You'll notice that some imports have different colors (L12, for example), and also the decorators (@Component(), @Input(), etc.) are different as well. Same with the class name and interfaces in the class declaration.

Is this an issue with nord-vim? Or maybe some limitation in treesitter? Please advice. Thanks!

crispgm commented 3 years ago

@joeveiga I think with tree-sitter the color would be more accurate. As you have shown in the screenshot, different kinds of keyword were colored differently. I think this is a feature.

@arcticicestudio for me, the elder version and current version (with tree-sitter) are not designed to be very distinguishable. Is it possible to make some improve on this? I forked Nord-vim and make one by myself:

Imgur

vdcow commented 3 years ago

@joeveiga It happens because developer of a tree sitter parser decided to highlight all camel case words started from the capital letters as a type

https://github.com/tree-sitter/tree-sitter-typescript/blob/fd08586b72f2ba8776d865b32aca561285c10cfd/queries/highlights.scm#L6

arcticicestudio commented 3 years ago

Hi @joeveiga, @crispgm and @vladimir-didenko :wave:, thanks for your contributions :+1: Thanks for your patience, I was on vacation for the last few weeks and I deliberately ignored the "digital life" during this time 😄

This is what I worried about after adding tree-sitter support: Loosing consistency due to “auto-magical“ mapping of syntax elements without dedicated highlighting groups. With Vim's "traditional" syntax parsing and language plugins it was easier for theme authors to control which elements gets highlighted, i.e. the dedicated jsFuncCall highlighting group is easy to understand and scoped to functions that are called like foo.bar(), but tree-sitter handles this on its own withput providing a dedicated group for such calls. Taking functions calls as an example tree-sitter works better while "traditional" highlighting often failed to detect functions which can be seen quite well on the GIF. On the other side it looks like elements like annotations/decorators are not handed by the TSAnnotation group which is also defined by Nord with nord12 as foreground color, but on the GIF it looks like nord8 is used instead.

@crispgm Which side is the one highlighted by tree-sitter on your screenshot? The right side looks like there are too many elements highlighted with nord9 that should only be used for elements like keywords and operators. The left side looks more like Nord's style, but there are also some elements that are highlighted with the wrong colors like the names of the enum (nord13 instead of nord4). Do you refer to these defintions of your fork? Looks like all groups are defined, even when they are already linked by default which is why Nord only overrides specific groups and link them to existing groups.

One problem I see here is that tree-sitter might apply the highlighing groups different depending on the language, i.e. the classes from @joeveiga's GIF should not use nord8 but nord7 like the correctly colored DotItem struct from @crispgm's screenshot. I think there is currently no other way than starting to compare the results between tree-sitter and "traditional" highlighting for popular languages and start to play around with the groups to see which are applied for which element. At the moment I don't have a fully running Neovim setup (yet) because I don't use Vim as my IDE, but I guess using nvim-treesitter/playground will help a lot to debug and find solution to these inconsisten highlighting problems.

crispgm commented 3 years ago

@arcticicestudio the left one is tree-sitter from my fork and the right one is official nord-vim. After implementing tree-sitter highlighting, nord9 is basically dominating and it's not as distinguishable as either non-tree-sittered one or my fork (which may regarded as a more distinguishable alternative).

arcticicestudio commented 2 years ago

Thanks for your patience regarding a reply, it's been busy times again. I want to refer to my comment in the arcticicestudio/nord#157 issue that clarifies my decision about creating a dedicated Nord port for Neovim. This will allow to handle highlighting and any other Neovim specific feature significantly easier when being able to use native Lua APIs instead of being bound to VimScript possibilities only. I'd like to keep this issue open until the new port is available so it can be used as reference when all Neovim specific code will be removed from this port like explained in another comment in the arcticicestudio/nord#157 issue. This will allow use to get rid of the inconsistent highlighting for a technology that is not native to Vim anyway.

jan-xyz commented 2 years ago

[...] a dedicated Nord port for Neovim. This will allow to handle highlighting and any other Neovim specific feature significantly easier when being able to use native Lua APIs instead of being bound to VimScript possibilities only.

Are you aware that you can use Lua inside of vim script with the Lua native API? like

if has('nvim')
  lua << EOF
    vim.api.nvim_set_hl(0, "@identifier", { link = "Identifier" })
  EOF
endif

I am also not sure if the situation changed now a bit with the latest developments in the nvim-treesitter repo. The old TS... highlight groups got removed and are replaced by the native treesitter groups like @keyword or @function with fallbacks to the Neovim standard highlight groups

svengreb commented 1 year ago

Thank you for your patience! 🙏🏼 It‘s been a while since I had free time to focus more on Nord, and my open source projects in general, and invest time in this issue due to work-life balance.

I recently published the first “Northern Post — The state and roadmap of Nord“ announcement which includes all details about the plans and future of the Nord project, including the goal of catching up with the backlog. This issue is part of the backlog and therefore I want to triage and process it to get one step closer to a “clean state“. Read the announcement about reaching the “clean“ contribution triage state in Nord‘s discussions for more details about the goal.

Therefore it has been added to the backlog in the central and single-source-of-truth project board that is also described in more detail in the roadmap announcement.


The further procedure with this issue is to continue to pursue nordtheme/nord#157 regarding the dedicated Neovim port and deprecations of Neovim-specific implementations in this port like mentioned in my previous comment.

@jan-xyz Thanks for the hint regarding the removed TS highlight groups! This could already help to improve the implementations when it leaves the task to Neovim instead of tree-sitter intervening in the highlighting itself. I‘m aware of the capabilities of Vim and Lua, but running a port in kind of “mixed“ format produces a lot of technical debt and also prevents all the other really good and unique features that Neovim provides, next to the way more native Lua integrations and APIs.

@crispgm joeveiga Also thanks again for your contributions! Would be coll if you could check if the changes in tree-sitter that @jan-xyz mentioned helped to get closer to the highlighting that nord-vim provides without tree-sitter.