Skip to content
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

Include generated files in repo #149

Closed
diktomat opened this issue Apr 5, 2022 · 5 comments · Fixed by #155
Closed

Include generated files in repo #149

diktomat opened this issue Apr 5, 2022 · 5 comments · Fixed by #155

Comments

@diktomat
Copy link

diktomat commented Apr 5, 2022

I understand your reasons for not including them, however, lots of editors configure tree-sitters by git url, which of course isn't possible if it's not in the repo. Examples being Helix (issue) or Nvim, both supporting local files too, but that of course is an extra step to take and keep up to date and can't be included in default configurations.

@alex-pinkus
Copy link
Owner

alex-pinkus commented Apr 5, 2022

Nvim seems to do ok with things as they are, and this is pretty clearly the direction that the tree-sitter folks want to take things. I’d rather invest the effort in getting Helix to a state where it can handle this. Is there a good explanation somewhere of what’s still needed for that?

@alex-pinkus
Copy link
Owner

As a side note, nvim-treesitter’s ability to regenerate parsers from source also lets them take advantage of query optimizations that require an ABI bump, which I implemented in tree-sitter specifically to benefit Swift. If we pregenerated the parser files we wouldn’t be able to take advantage of those until every consumer was on ABI 14.

@the-mikedavis
Copy link

For Helix to not depend on generated parser files checked into source: helix-editor/helix#1970

Generating the parser files with tree-sitter-cli is not a good fit for Helix. I think a runtime or build-time dependency on tree-sitter-cli flies in something like nvim-treesitter because tree-sitter is the main feature, but for tools where tree-sitter is an implementation detail (semgrep and difftastic for example), it's an undue hassle for users and packagers.

On the one hand I detest checking in generated src and I agree tree-sitter consumers should not assume they're checked in. OTOH it's a pragmatic and low-cost way to make it easy for the majority of tree-sitter consumers to package the grammar. It also doesn't stop anyone from using a non-default ABI: eager consumers would just need to re-generate the parser.

@alex-pinkus
Copy link
Owner

That’s a fair point.

To me the biggest concern with the generated files is that they make pull requests more likely to have merge conflicts. Could we maybe compromise by having a separate branch with generated source? GitHub Actions could trigger an automatic checkin every time we publish a new release. That way the diffs would stay clean and there wouldn’t be too many new large deltas in the repo, but the system would still look like a git-based tree-sitter parser.

@alex-pinkus
Copy link
Owner

All right, this has now been done! The with-generated-files branch of tree-sitter-swift gets automatically updated every time I publish a new release (actually, any new tag). I also published 0.2.0 to seed it with an initial set of static files. @d12bb @the-mikedavis FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants