search

claytonrcarter / tree-sitter-phpdoc

PHPDoc grammar for tree-sitter
22 stars 10 forks source link

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

Closed amaanq closed 3 months ago

claytonrcarter commented 3 months ago

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 commented 3 months ago

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)

claytonrcarter commented 3 months ago

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