sourcegraph / tree-sitter-jsonnet

tree-sitter grammar for JSONNET
MIT License
15 stars 4 forks source link

refactor: cleaner $.expr + match order with spec #14

Closed Duologic closed 1 year ago

Duologic commented 1 year ago

First of all, great job on this tree-sitter parser. This is the first time I'm working with tree-sitter, so please be kind.

I'm debugging a few issues but had a hard time navigating the code so I spent a bit of time refactoring first.

This PR does 3 things:

I've maintained the visibility by using the underscore prefix on all new items but we might want to make them visible in the output (and perhaps hide $.expr as it's ubiquitous) in a follow up PR.

Duologic commented 1 year ago

ping @tjdevries @aryx Any feedback on this?

aryx commented 1 year ago

Would be good to have some CI checks on this repo so at least we're making sure it does not break the build.

Duologic commented 1 year ago

Fair point, I have run the test cases locally tho. I have several changes on my fork that I'd like to pass along to this repo, I'll have a look at adding CI before those.

aryx commented 1 year ago

maybe @tjdevries should give some 'maintain' right to one of us, otherwise we will have to fork the repo to get improvements in it.

aryx commented 1 year ago

I've created https://github.com/sourcegraph/tree-sitter-jsonnet/issues/17

Duologic commented 1 year ago

@tjdevries Any feedback on this? (last ping, no worries if you are too busy, we can fork and converge later)

tjdevries commented 1 year ago

Hey, thanks for the ping! I will follow up to see about adding other maintainers (may have to move it out of sourcegraph org, not sure).

in the meantime, PR looks good to me. I'll merge. Thanks!