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

Keep repo files in-line with other grammars #122

Merged
merged 6 commits into from
May 5, 2024
Merged

Conversation

amaanq
Copy link
Member

@amaanq amaanq commented May 5, 2024

I'm not sure what the tools, extra stuff in test besides corpus files, and flake stuff are, but they're not needed and trying to run some of the custom "tests" didn't even work - so simply get rid of it.

It can be added back if it's usable via the cli - parser tests and query tests can be done so with the cli, so I'm not sure what this was for.

Also updated the scanner to use the alloc and array headers for relevant operations, as well as streamline the workflows + correct bindings that needed to specify the scanner file.

amaanq added 6 commits May 5, 2024 03:11
These should be implemented using the standard tree-sitter testing
framework for corpus files or highlight/query tests.
@amaanq amaanq merged commit fd310ff into tree-sitter:main May 5, 2024
4 checks passed
@tek
Copy link
Contributor

tek commented May 5, 2024

you have got to be kidding

guess that's me being done with this org. if someone wants future changes, you'll have to cherry-pick from my personal account

cc @clason

@clason
Copy link

clason commented May 5, 2024

Sorry about that; I have nothing to do with these changes. (I'm just a downstream maintainer of nvim-treesitter and not related to this org. I just wanted to pick your brain on the other PR.)

@tek
Copy link
Contributor

tek commented May 5, 2024

I know, just notifying you about moving my maintainership to a different repo, in case it makes a difference for nvim-treesitter

@amaanq
Copy link
Member Author

amaanq commented May 5, 2024

you have got to be kidding

guess that's me being done with this org. if someone wants future changes, you'll have to cherry-pick from my personal account

cc @clason

Hey @tek, I probably should have waited for you to comment on this before merging, so I do want to genuinely apologize to you for not doing so. I felt a little slighted by your choice to continue developing on main while not checking in parser.c (hence why I wanted to comment on your PR before merging when it was marked ready, but that only lasted a day or so). I totally get where you're coming from with that, but every grammar upstream (in this org) has a convention about this currently, and that is to just check in parser.c. Had you asked me or Max, we do have plans to get rid of the need for this eventually, as noted in Max's 1.0 checklist, but the tooling and infra is not there yet. We do want to move to GH releases for parser.c artifacts, and the CLI will be augmented to handle this for authors pretty well, but again, we're just not there yet.

I would really like to continue working with you on developing the grammar, and I know you've done a fantastic job thus far maintaining it - so I'm really sorry if I offended you with what I did here (late night tiredness + wanting to get it in, yeah bad move). I do want to ask though, a lot of these files did not seem to function for the repo itself, but rather for you for local development/testing? At least the rust crates seemed like they were a bit unnecessary and the parse-test one didn't pass successfully as well. The tools stuff I'm not so sure about, and I probably should've waited to ask you.

Are you on Matrix or some related messaging platform by any chance? I'd love to continue chatting there and make amends, I really do want to coordinate development better with you so that consumers from every angle can benefit from your work, that's really my main goal 😁, but if you choose to leave and develop in your own fork, well, I can't stop you, but it'd be a shame

@tek
Copy link
Contributor

tek commented May 6, 2024

since you appear to have the intention to make grammar repos uniform, it just makes sense for development to happen in a separate environment.

so best I can offer is to mirror changes back to this repo – I can't work if there's a chance that all of my tooling is just deleted

@ObserverOfTime
Copy link
Member

Honestly, some of the tests can be added back with some refactoring: rust tests should be moved to lib.rs (using format!("{:#}", root_node) instead of the custom function) and parse tests should either be added to the corpus or moved to examples and parsed in CI. Arbitrary queries don't have an official method of testing quite yet.

The tooling can be added back if documented, otherwise gitignored and used by you locally if no one else needs it.

@sergv
Copy link

sergv commented May 15, 2024

@amaanq I'm just trying to form full picture about future of this repo and where to find tresitter Haskell grammar. Given that you made sweeping changes is it to be expected that you'll maintain the grammar going forwards, fix existing bugs/inconsistencies/etc?

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.

5 participants