claytonrcarter / tree-sitter-phpdoc

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

Upgrade to latest TS version #2

Closed mikehaertl closed 2 years ago

mikehaertl commented 2 years ago

I tried to add your parser to nvim-treesitter but got version issues:

ABI version mismatch for phpdoc.so: supported between 13 and 13, found 12

I've not worked on parsers before so not sure if I followed the right path:

I've also bumped the treesitter-php version.

claytonrcarter commented 2 years ago

Thank you for this. Can you help me w/ some strategy, though? This repo was originally created for my PR to add tree-sitter support to Atom's language-php, and Atom is still using TS 0.17. So, merging this as is would break the repo/code for my main goal. There is a some (but just some) movement to update Atom to TS 0.19, but it appears stalled.

I would like to be able to make both of us happy, but until Atom is updated, I think we might be at impasse. How will this code be consumed in nvim? Does it need to be packaged on NPM, or is it included via git repo or something? I'm wondering if we could just maintain parallel branches: 1 for "legacy" or pre-0.17 tree-sitter, and 1 for current tree-sitter.

What do you think?

mikehaertl commented 2 years ago

How will this code be consumed in nvim?

From what I've seen the repo is cloned to a temp dir and then parser.c is compiled into a .so file which is then used by the nvim plugin.

Two branches sound good. I'm just not sure if a specific branch can be configured with nvim-treesitter. It defaults to master but I'll have a look.

mikehaertl commented 2 years ago

Ok, we can lock the dependency to this repo to a specific version in nvim-treesitter. So if we have 2 branches it would at least work for now.

But IMO it's not a good long term solution as all changes have to be applied to both branches. And it doesn't auto-update the changes in nvim-treesitter as the revision must be bumped.

mikehaertl commented 2 years ago

I just learned that we can basically do a tree-sitter generate when installing a parser in nvim-treesitter. This then generates an up-to-date parser.c. So problem solved. This change is no longer required.

claytonrcarter commented 2 years ago

Glad you figured something out. If that turns out to be too much work, just let me know and we can try again.

But IMO it's not a good long term solution

Oh, yeah, it's a terrible option, but until Atom get's an update, we're stuck needing to support tree-sitter ~0.17, so my hands are a little bit tied up.

Anyway, thanks!

clason commented 2 years ago

Turns out all that is needed to make this build is to update the nan dependency from 2.14.0 to 2.15.0. Is there any reason not to bump this version?

marioy47 commented 2 years ago

Since this is the first result after a google search, I'll leave this here (hope that's ok)

Using node 14 solves the issue on a Macbook M1.

If you are using nvm then just

nvm install 14
nvm use 14
npm i -g npm
nvim +"TSInstall phpdoc"