dracula / vim

🧛🏻‍♂️ Dark theme for Vim
https://draculatheme.com/vim
MIT License
1.33k stars 455 forks source link

Update treesitter highlight groups #291

Closed yuys13 closed 1 year ago

yuys13 commented 1 year ago

nvim-treesitter has removed TS* highlight groups. Thus we need to use @foo.bar style highlight groups.

see https://github.com/nvim-treesitter/nvim-treesitter/pull/3656

benknoble commented 1 year ago

Is this a version-specific difference? Should we make any effort to maintain backwards compatibility between the two sets of groups? @dsifford

yuys13 commented 1 year ago

I remove TS* highlight groups for several reasons.

There is no version for nvim-treesitter. I am guessing that many users are using the HEAD commit of nvim-treesitter.

We need to use the old nvim-treesitter to take advantage of TS* highlight groups. We need to use the old nvim-treesitter with a commit hash. If we use dracula theme that matches old nvim-treesitter as well, we can fix the commit for dracula theme as well. However, I assume that most people use the latest version of both dracula theme and nvim-treesitter. I couldn't think of any need to deal with the case where someone uses an older vim-treesitter and wants to use the latest dracula theme.

I also think it would be difficult to change the behavior with different versions of nvim-treesitter.

My concern was if we keep TS* highlight groups, how long will dracula theme support it? nvim-treesitter has already dropped support.

Please let me know your thoughts.

yuys13 commented 1 year ago

I noticed that some default values were lost due to breaking changes in nvim-treesitter. I will work on restoring the lost default values out of the highlights that I removed in the following PR. https://github.com/dracula/vim/pull/232

yuys13 commented 1 year ago

Lost default values were added. If changes are needed for backwards compatibility, I would be willing to work on them.

benknoble commented 1 year ago

I couldn't think of any need to deal with the case where someone uses an older vim-treesitter and wants to use the latest dracula theme.

This situation is as simple as updating one plugin and not the other, and now things look broken. We will probably get lots of reports.

We could probably keep the old groups for a period of time (say, 6 months? a year?) and mark them as deprecated (where?) with a link to update nvim-treesitter, then remove them.

yuys13 commented 1 year ago

We will probably get lots of reports.

I see, indeed. Certainly no need to rush to remove the TS* highlight groups, so I put these back in. With this change, both the old nvim-treesitter and the new nvim-treesitter can be supported.

yuys13 commented 1 year ago

I realized that in order to support older versions of nvim-treesitter as well, I would also need support for older neovim. Therefore I have modified nvim-treesitter highlighting to work even if the neovim version is 0.8 or lower.

ttytm commented 1 year ago

Something new on mergin this to get treesitter related highlights back up?

benknoble commented 1 year ago

Mostly waiting on @dsifford, since I don't NeoVim.

benknoble commented 1 year ago

Very sorry for the delay! Got laid off last week and have been busy trying to get back in the saddle.

Sorry to hear that! Hope everything goes well for you.

Looked over the PR. This looks good to me. We can merge and address anything that comes up if that happens later. 👍

I can handle this.

adamdottv commented 1 year ago

This good?

benknoble commented 1 year ago

Done!

yuys13 commented 1 year ago

Thank you all! I hope all will be well. :pray: