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

chore: add example parsing script #53

Merged
merged 5 commits into from
Feb 14, 2024
Merged

Conversation

calebdw
Copy link
Contributor

@calebdw calebdw commented Feb 2, 2024

Hello!

This adds an example parsing script similar to other tree-sitter repos. The script is used to checkout repos and execute the grammar on the files to ensure there's no issues parsing files in the community. Currently this is just parsing Breeze, but we can add other popular repos as needed/desired.

This actually revealed two bugs (I'll open an issue):

  1. @js is parsed as a conditional
  2. No support for casting @foreach((array) $messages as $message)

Thanks!

@calebdw
Copy link
Contributor Author

calebdw commented Feb 2, 2024

Note that you can ignore errors by adding them to the known-failures.txt file, so there's nothing keeping this pr from being merged

@EmranMR
Copy link
Owner

EmranMR commented Feb 2, 2024

@calebdw amazing. This is actually very neat!
I am also definitely up to add more repos to cover as much as possible!

One question though. may I ask how do you handle your conflicts with the tree-sitter-php parsers 🤔? so far the only way I could get this working was removing the tree-sitter-php from the path that was pointed to at config.json for tree-sitter. That was the main reason for naming the playground example file example.blade .

I have actually already raised an issue for this a while ago, as it was bugging me during the dev

@calebdw
Copy link
Contributor Author

calebdw commented Feb 2, 2024

I'm not sure what conflicts you're talking about? I haven't really had any issues...

@EmranMR
Copy link
Owner

EmranMR commented Feb 4, 2024

@calebdw ah, let me elaborate more.

So let us say you have a folder where all you tree-sitter-x parsers live, and your tree-sitter per user configuration is pointing there. The conflict happens if you have tree-sitter-blade and tree-sitter-php both in the same folder.

When you run tree-sitter parse which is the case in the bash script, conflict occurs because both tree-sitter-blade and tree-sitter-php hit .php extension. see here. To be honest the .php extension makes no sense as it is an old artefact from the early dev phase, I was trying to make up .blade.php and that was not possible based on the issue in my previous comment. Ever since then I was mainly using the .blade for debug parsing etc.

The Parse Error Example

  • So when I first ran this, I was getting an error that no grammar.js is found or the file does not exists (confusion on which parser to use)
  • I can not reproduce that because I deleted the tree-sitter-php and now I do not get the error after reinstalling. 🤷‍♂️
  • After cloning the tree-sitter-php again, I am getting the stuff parsed in the example/. But the results are an illusion! It is using the tree-sitter-php instead of tree-sitter-blade

To reproduce

  1. Have both parsers in the folder
  2. remove the stuff in the known-failures.txt
  3. then run the script
  4. As shown in the image you will get 100% and no issues to report
  5. Now delete the tree-sitter-php or move
  6. Run the script again and you will get the correct parsing with the errors.
Screenshot 2024-02-04 at 10 45 30

Fix?

  • All I can come up with to avoid confusion, is adding another for loop in the bash script that changes all the .blade.php into .blade ?

I am actually surprised that you managed to parse with no issues, considering that you are working with/on both parsers 🤔 Would be interesting to know how and why! :)

@calebdw
Copy link
Contributor Author

calebdw commented Feb 5, 2024

Hmm, I've never run into any issues, and I've never touched the default treesitter config so just running with the defaults. I install all of my repos at ~/sources and all of my treesitter repos go under ~/sources/treesitter, so I have:

  • ~/sources/treesitter/tree-sitter-blade
  • ~/sources/treesitter/tree-sitter-php
  • ...

and I've never had any conflicts

@EmranMR
Copy link
Owner

EmranMR commented Feb 5, 2024

Mmm that is so strange now! Your set up is pretty much like mine. So with this same set up if you:

  • Delete the content of the known-failures.txt
  • and then run the script.
  • you will correctly get the parse errors? 🤔

Can I ask your OS? Are you on Mac or Linux?

@calebdw
Copy link
Contributor Author

calebdw commented Feb 5, 2024

I tried that and still no issues. I'm on Linux (PopOS from System76)

script/parse-examples Outdated Show resolved Hide resolved
@EmranMR
Copy link
Owner

EmranMR commented Feb 14, 2024

@calebdw sorry my comments were sitting in the files as "pending", I thought it was "pending" awaiting your review!

What an awful UI/UX design by github! However I went to the repo, when I was not logged in and realised it has not been submitted. Anyhow apologies for super delay getting back to you, this should have reached you a week ago maybe more! 😬

@calebdw
Copy link
Contributor Author

calebdw commented Feb 14, 2024

No worries!

I can try to add the --scope, but I'm not sure if that will work in the actual ci (the repo might not be in the ts path). If that's the case and there's a conflict, then the script should prefer ci use over local use for the time being

@EmranMR
Copy link
Owner

EmranMR commented Feb 14, 2024

It should hopefully be fine in the CI 🤞, as it is based in the data in the package.json!

Of course if not we can revert etc as CI is more important. I will scramble an adjusted one maybe for my own local testing :)

I usually like to heavy stress test locally before I commit and this is a great tool to have in the toolbox!

It was nice to see new parsing issues arise after your first run using only one repo!

@EmranMR EmranMR merged commit 4ea4026 into EmranMR:main Feb 14, 2024
1 check passed
@EmranMR
Copy link
Owner

EmranMR commented Feb 24, 2024

but I'm not sure if that will work in the actual ci

you were indeed right! I just revert it back to your original!

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