-
Notifications
You must be signed in to change notification settings - Fork 18
include generated files #245
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
Conversation
nvim-treesitter would also like this to get merged! :)
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.
Hello thanks for this PR should be very usefull :)
f8b43c6 to
d1697ee
Compare
Our motivation for keeping generated files out of the repo is aligned with this explanation from the swift parser's maintainer.
Neovim doesn't need generated files at all; it can generate them itself. Please look at the installation instructions on the readme.
Regarding helix, I personally much prefer what the Swift parser did, which is to have a branch that is automatically updated with the generated files whenever the main branch changes. (via a GH action.)
Is anyone interested in implementing copying that here?
clason
commented
May 27, 2024
Neovim doesn't need generated files at all; it can generate them itself.
Well, that's not strictly true. The nvim-treesitter plugin can call tree-sitter to generate the parser before compiling it, but only if the user has the tree-sitter CLI installed (which is not a hard requirement since the vast majority of grammars do commit the parser).
And it is true that upstream recommends not committing the parser iff you make versioned releases which do contain all generated files. I (and upstream) strongly recommend against having a separate branch for that; that just creates confusion and additional work. (The swift parser is not a good model to follow in general; they have very specific needs and workflows.)
Even if you don't keep the parser in the repo, though, you should update and fix the files you do track (bindings, headers, an in particular the scanner includes).
Ok so we'll probably merge this, but we would like a GH action to ensure generated files are up to date. @aspeddro Pointed me to this example on the rescript-core repo. @FreddieGilbraith I'd like to take you up on your offer of adding the action, if you're still interested.
Just one question: Is it necessary to include the bindings? AFACT from looking at @ribru17's PR in nvim-treesitter, neovim doesn't need them. Is that the case for Helix too?
If not, we can generate the files with --no-bindings and end up with smaller diffs.
clason
commented
May 30, 2024
nvim-treesitter needs no bindings (Helix might need the Rust ones, I don't know), but these of course are not the only consumers. I can't speak for any of them, though.
2460004 to
6693780
Compare
6693780 to
8384d07
Compare
I've added a CI check, and a git hook script to make sure generation is as part of commits.
Looks like no one needs the bindings, helix seems to work without them :)
Thank you @FreddieGilbraith!!!
per the discussion here: rescript-lang/tree-sitter-rescript#245 it looks like its generally not needed, the only thing needed is the src/ output
Some projects require the generated files to be present in a tree-sitter repository to use them. Noteably the helix editor is currently refferencing a fork of this grammar and therefore can not be updated to the newest version.
Please can we include the built files in the repo.
I've not included a Github Actions check to ensure the generated files are up-to-date, but am happy to add one if you'd like.
Thanks :)