-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "vimPlugins.nvim-treesitter: collate grammars" #341079
Conversation
This reverts PR NixOS#319233. It caused a lot of weird regressions and a lot of headaches for me. Even a 20% improvement isn't worth it. Also see NixOS#319233 (comment)
A NixOS#319233 accidentally reverted NixOS#321550. Last one caused a very annoying regression to any Nix user (see NixOS#332580). I suppose this is a bug in upstream grammar, so I workaround it this way until it is properly resolved.
This is a straight up revert, yes? Im ok with that. It does cause weird things when multiple things add grammars because now you have 2 bundled grammar balls with the same name instead of just 1 The workaround added is ok I think? You likely know more about if thats a good solution or not than I do right now. Id like to see a fix get pushed to the nix grammar eventually for that. |
Result of 2 packages built:
|
Apologies again and thanks for handling this @PerchunPak! For posterity, what do #332580 and #321550 have to do with #319233? |
Yes, it is
Treesitter should prefer the first grammar it finds, as stated in
That's for upstream to work on, I don't know anything about writing queries for treesitter |
LGTM then! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of changes
This reverts PR #319233. It caused a lot of weird regressions and a lot of headaches for me. Even a 20% improvement isn't worth it. Also see #319233 (comment)
In addition, I added a workaround for #332580 as it is also a very annoying regression that will reappear after merging this (minimal reproduction example: #332580 (comment))
Fixes #341442
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)CC @figsoda, @BirdeeHub
@MangoIV, could you try this with koka? Treesitter didn't detect its grammar for some reason for me
Add a 👍 reaction to pull requests you find important.