murtaza64 / tree-sitter-groovy

Tree sitter parser for groovy
MIT License
14 stars 10 forks source link

feat: Implement labels #14

Closed HendrikJanssen closed 1 month ago

HendrikJanssen commented 2 months ago

fixes #13

I did add the optional(label) to the statement sequence, also added expression statements since these were missing?

I am not really sure what happened with the for tests. From a glimpse, these were wrong before? A single increment_op was what I was expecting, not sure where the assignment came from beforehand, maybe someone can enlighten me what happened here?

I did not add labels to the break right now since I wanted to keep the changes minimal to be better to review. I will add these and quoted identifiers once I am more familiar with tree-sitter and have gathered feedback from this PR.

murtaza64 commented 1 month ago

The way I designed the grammar I didn't want a plain _expression to count as a _statement. Not sure off the top of my head if this breaks anything... but if the tests aren't broken, I think it should be fine?

Adding _expression as a _statement explains why the for tests changed--an assignment node is inserted to promote i++ from an expression (increment_op) to a statement

murtaza64 commented 1 month ago

Feel free to try implementing the rest of the label feature, as long as the tests stay passing and highlighting looks fine on typical code files I'm happy to merge it

murtaza64 commented 1 month ago

If you fix the indentation I can merge this as-is

HendrikJanssen commented 1 month ago

Im going to come back to this PR on the weekend, I have much to do this week. But thank you very much for your time and explanation!

murtaza64 commented 1 month ago

I managed to fix the indentation with the python script I wrote to help with the tests (python treesexpy/ts_test_helper.py corpus/label.test which diffs the output from treesitter with the test and shows you a line by line diff, optionally updating the test)

murtaza64 commented 1 month ago

Thanks for the PR!