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

Add GDScript Support w/ tree-sitter-gdscript #93

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

rinOfTheStars
Copy link

As mentioned a while ago in #76, I'm unable to get yarn to behave for long enough to actually add https://github.com/PrestonKnopp/tree-sitter-gdscript to the project. I've done all of the other required steps that don't involve yarn, however, so I just need someone to run yarn add -D tree-sitter-gdscript and then yarn compile to get this moved along.

@PrestonKnopp is the dev for the tree sitter plugin being added here, so I'm pinging them for posterity's sake—afaik the plugin is feature complete with the version of GDScript in Godot 4.3 (the current stable release) but I might be wrong on that front; please do let me know if I am.

@PrestonKnopp
Copy link

Hi, I have just updated tree-sitter-gdscript to use the latest tree sitter. Additionally, I ran tree-sitter init --update to pull in the new (to tree-sitter-gdscript) default package configuration files.

Prebuilds are not set up yet. I'm not sure if that affects you.

Let me know if I can amend tree-sitter-gdscript in a way to improve this workflow.

@rinOfTheStars
Copy link
Author

That shouldn't be an issue afaik—the issues are with other treesitter plugins not building due to dependency version requirement jank and some of them outright refusing to build on macOS. Thanks anyways!

Got running `yarn` to work by using an older node ver; `yarn compile` still fails however
@rinOfTheStars
Copy link
Author

Now able to get to the point of running yarn (by using an older node ver.), but running yarn compile fails due to an issue with tree-sitter-agda (apparently lacks a tree-sitter section in its package.json?); not sure if this is something that needs to be fixed upstream or what unfortunately

@pokey Does yarn compile need to be run before a PR is ready to go, or is that just for building the final product that gets shipped w/ cursorless? If it's the former, this PR should be ready for review and merging, afaik

@pokey
Copy link
Member

pokey commented Jan 7, 2025

hmm i'm getting the following error when trying to build locally:

npx tree-sitter build --wasm node_modules/tree-sitter-gdscript/
Failed to locate a package.json file that has a "tree-sitter" section, please ensure you have one, and if you don't then consult the docs
make: *** [parsers/tree-sitter-gdscript.wasm] Error 1
error Command failed with exit code 2.

@rinOfTheStars
Copy link
Author

I'll try updating the tree-sitter-gdscript to latest commit, just in case PrestonKnopp already fixed this, later today. If that doesn't work then I'll ping him since the issue is presumably in that plugin itself.

@pokey
Copy link
Member

pokey commented Jan 7, 2025

Already tried that on current tip of default branch

@rinOfTheStars
Copy link
Author

Based on some digging I've done in other tree-sitter parser projects, that seems to be an error related to outdated versions of the tree sitter CLI, as they used to insert a tree-sitter block into package.json, something that is no longer the case.

I've pushed an update to both the tree-sitter-gdscript and tree-sitter commit hashes to update them to latest commit and ver 0.24.6 respectively; try building again with both of those?

@pokey
Copy link
Member

pokey commented Jan 9, 2025

Hmm that didn't seem to help:

➜ npx tree-sitter --version
tree-sitter 0.22.6 (b40f342067a89cd6331bf4c27407588320f3c263)
➜ npx tree-sitter build --wasm node_modules/tree-sitter-gdscript/
Failed to locate a package.json file that has a "tree-sitter" section, please ensure you have one, and if you don't then consult the doc

From what I can tell 0.22.6 is the latest version of the tree-sitter CLI

@rinOfTheStars
Copy link
Author

It isn't, the latest is 0.24.6:

% npx tree-sitter --version
tree-sitter 0.24.6 (21a517c423010811147b0b1aa1e7aedc39ce91aa)

Running npx tree-sitter build --wasm node_modules/tree-sitter-gdscript/ on my machine generates a new error:

Failed to find `tree-sitter` section in package.json, unable to migrate

which is baffling, considering tree-sitter-gdscript is built against a version of tree-sitter from after that package.json section was removed. This is also what yarn compile errors on now, albeit on a different parser. This seems to suggest something seriously wrong somewhere in this project's buildscripts, but idk what?? At the very least it seems adding tree-sitter itself as a dependency to package.json causes everything to error in the same way...

@pokey
Copy link
Member

pokey commented Jan 9, 2025

Hmm yeah unfortunately the whole setup in this repo is a bit cursed. We've been limping until someone gets to cursorless-dev/cursorless#1488. If you're interested in trying to take a crack at that, that should make everything a lot smoother. Even just doing step 1 would be a win, and then as a temporary step we could just pull the wasms from that package into this repo to avoid needing to do steps 2 and 3 right away. If interested lmk and happy to discuss on discord if helpful

@rinOfTheStars
Copy link
Author

I'll give working on the first part of that issue a shot at the very least. Will take a bit since I need to update all of the tree-sitter plugin versions, alongside getting at least baseline understanding of typescript and github actions down...

@pokey
Copy link
Member

pokey commented Jan 10, 2025

@cocona20xx awesome lmk if you get stuck!

@pokey
Copy link
Member

pokey commented Jan 10, 2025

also fwiw we just removed the custom web-tree-sitter build infra and bumped to 0.24.6; I've merged those changes into this PR to save you dealing with the merge conflicts

@BlueDrink9 BlueDrink9 mentioned this pull request Feb 23, 2025
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.

3 participants