refactor: cleaner $.expr + match order with spec #14
Conversation
| @@ -5,3 +5,6 @@ generate: | |||
|
|
|||
| test: generate | |||
| ${ts} test | |||
|
|
|||
| fmt: | |||
| npx prettier --tab-width 4 -w grammar.js | |||
There was a problem hiding this comment.
I'm not a JS dev, so no clue if this one makes sense, I choose 4 spaces as it created only a small diff.
| number: ($) => $._number, | ||
| string: ($) => $._string, |
There was a problem hiding this comment.
I've linked $.number and $.string here so they show up in the right order but all the details can stay at the bottom to reduce the diff.
Duologic
changed the title
|
ping @tjdevries @aryx Any feedback on this? |
|
Would be good to have some CI checks on this repo so at least we're making sure it does not break the build. |
|
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. |
|
maybe @tjdevries should give some 'maintain' right to one of us, otherwise we will have to fork the repo to get improvements in it. |
|
I've created #17 |
|
@tjdevries Any feedback on this? (last ping, no worries if you are too busy, we can fork and converge later) |
|
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! |
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:
prettier$.exprinto separate items (and matched order with the Jsonnet spec, there were a few differences)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
$.expras it's ubiquitous) in a follow up PR.