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

fix: use grammar field to reference PHP rules, regenerate with latest master, remove reset function #32

Merged
merged 3 commits into from
May 7, 2024

Conversation

amaanq
Copy link

@amaanq amaanq commented May 5, 2024

No description provided.

@claytonrcarter
Copy link
Owner

Thank you! FYI it looks like the upgrade from tree-sitter-cli 0.20 → 0.22 required that the corpus tests to be moved to test/corpus. I've pushed a commit that does that, because otherwise CI was passing without actually running any tests.

With tree-sitter-cli 0.22, if I run ./node_modules/.bin/tree-sitter generate, I'm seeing a lot of changes that are not included in this PR (see below).

❯ git stat
## fix
?? .npmignore
?? TODO.md
?? foo.php
?? foo.phpdoc

❯ ./node_modules/.bin/tree-sitter generate
Replacing nan dependency with node-addon-api in package.json
Adding node-gyp-build dependency to package.json
Adding prebuildify devDependency to package.json
Adding an install script to package.json
Adding a prebuildify script to package.json
Adding peerDependencies to package.json
Adding types to package.json
Adding files to package.json
Updated build.rs with the /utf-8 flag for Windows compilation
Replacing index.js with new binding API
Replacing binding.cc with new binding API
Replacing binding.gyp with new binding API

❯ git stat
## fix
 M binding.gyp
 M bindings/node/binding.cc
 M bindings/node/index.js
 M bindings/rust/build.rs
 M package.json
?? .editorconfig
?? .gitattributes
?? .npmignore
?? Makefile
?? Package.swift
?? TODO.md
?? bindings/c/
?? bindings/go/
?? bindings/node/index.d.ts
?? bindings/python/
?? bindings/swift/
?? foo.php
?? foo.phpdoc
?? pyproject.toml
?? setup.py

Many of these look like project setup boilerplate, but some of them seem important (I think?): eg the replacement of nan with node-addon-api, several other changes to package.json, and numerous changes to the (already commited) bindings for node and rust in bindings, plus addition of other bindings.

[question] Do you have any comments or context on if those should be committed to the repo? Or should the generated files not be committed because they will/can be regenerated on demand by the cli?

@amaanq
Copy link
Author

amaanq commented May 6, 2024

yeah, they are important for adding support for other bindings, and in node-ts we switched from nan to napi. If you want, I can update this PR to account for those changes as well if you'd like and update whatever requires manual changes

(to answer briefly, yes they should be committed)

These are all generated by `./node_modules/.bin/tree-sitter generate` and should be under version control.

Ref: claytonrcarter#32 (comment)
@claytonrcarter
Copy link
Owner

OK, thanks for the clarification. I've added them all, and no manual changes seemed necessary to keep tests passing. 👍

@claytonrcarter claytonrcarter merged commit 5f9d2da into claytonrcarter:master May 7, 2024
2 checks passed
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 this pull request may close these issues.

2 participants