tree-sitter: make tree-sitter-grammars an overridable scope #537058
Conversation
7392927 to
aaae998
Compare
ddce663 to
2299500
Compare
RossSmyth
left a comment
There was a problem hiding this comment.
I understand better on how it is structured, there was/is is scope, but it has no relationship to allGrammars and grammars so overrides don't actually do anything. Few observations and questions.
| # `.allGrammars` list of non-broken grammar derivations | ||
| # `.withPlugins` build a grammar link farm | ||
| */ | ||
| grammarsScope = lib.makeScope newScope ( |
There was a problem hiding this comment.
Should probably just be called grammars, no need to make people wonder "what is a scope" if they have no need to be aware.
| grammarsScope | ||
| withPlugins | ||
| allGrammars | ||
| ; |
There was a problem hiding this comment.
The problem is that grammars and grammarScope are not actually related here. If I were making it, I would only expose grammar, which would be a scope.
Then you need to put a fix between allGrammars and grammars so that when grammars is overriden, allGrammars changes too. Currently it does not as you just make a copy of the unoveridden scope.
The above is similar to how python3Packges and typstPackages works, just without allGrammars.
Can alias grammarsScope to grammars so it's not a breaking change.
| passthru = { | |
| inherit | |
| buildGrammar | |
| builtGrammars | |
| withPlugins | |
| allGrammars | |
| ; |
There was a problem hiding this comment.
I think there are two reasonable priorities here.
Using tree-sitter.grammars for the built scope matches the Python/Typst package-set shape and makes the public extension point tidy. The cost is that tree-sitter.grammars currently exposes source specs, so external consumers using that passthru could break.
Keeping tree-sitter.grammars as source specs and putting the built scope at something like tree-sitter.grammarPackages is a little less elegant, but it preserves the existing public meaning while still letting tree-sitter-grammars = recurseIntoAttrs tree-sitter.grammarPackages and making overrides compose.
Given the chance of external consumers, I lean toward the backwards-compatible shape unless we think the tree-sitter.grammars source-spec passthru is safe to repurpose.
2299500 to
7a301dd
Compare
pkgs.tree-sitter-grammars was a flat recurseIntoAttrs of builtGrammars. Point it at the existing grammarsScope so it carries .overrideScope, and add grammar-only views to that scope — derivations (attrset), allGrammars (non-broken list), and withPlugins — so it stays iterable alongside the package-set helpers. builtGrammars remains the stock, non-overridden set.
Read grammars from the public tree-sitter-grammars scope, via its derivations and allGrammars views, in emacs treesit-grammars, diffsitter, the python bindings, and nvim-treesitter, instead of tree-sitter.builtGrammars. An overrideScope override of the set now propagates to these packages.
Apply Helix's locked grammar overlay to pkgs.tree-sitter-grammars so global grammar-scope overrides compose through the public extension point.
Document overrideScope as the supported extension point and the scoped grammar-only views (derivations, allGrammars, withPlugins) that consumers should iterate.
7a301dd to
0a53614
Compare
✅
|
aciceri
left a comment
There was a problem hiding this comment.
I like this, I wish it had been already here when I packages helix's grammars, happy to see that someone implemented it!
No derivations are rebuilt so we are sure that it wouldn't cause any harm to grammars consumers, in my opinion it's already good enough to be merged and further adjustments can be done in separate PRs.
Turn
pkgs.tree-sitter-grammarsinto the single, override-aware source of tree-sitter grammars, and route the existing consumers through it.Alternative solution to (technically we could combine them and use the other as a more "backwards-compatible" drop in.
Closes #536475
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.