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

feat: restructure repository for new tree-sitter versions, add flake.nix #47

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

ErinvanderVeen
Copy link
Collaborator

I was working on reviewing #46 when I wanted to add tests. Tree-sitter has changed a lot since we created this repository. This PR brings it up to date with what is expected by the tree-sitter tool.

@ErinvanderVeen ErinvanderVeen force-pushed the restructure branch 2 times, most recently from ed93776 to 4b8712e Compare May 31, 2024 11:59
@ErinvanderVeen ErinvanderVeen marked this pull request as ready for review May 31, 2024 12:02
Copy link
Contributor

@yannham yannham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of generated boilerplate, on which I can't say much because I'm not familiar with all the ecosystems. But in general looks ok.

One thing to do before merging is to update the README though, I think there are hacking instructions that are outdated - and maybe we can add some list of the bindings that we provide?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should use that instead of the manually defined C arrays in the scanner (in a different PR though, this one is already big)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should at least try I suppose

'';
};

checks.default =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use nix flake check instead of npm for the CI now? (it's an open question, and should be a different PR anyway)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! That is probably a good idea!

src/tree_sitter/alloc.h Show resolved Hide resolved
@yannham
Copy link
Contributor

yannham commented Jun 3, 2024

@ErinvanderVeen do you want me to take a look at the README update?

@yannham
Copy link
Contributor

yannham commented Jun 3, 2024

(I must say the tree-sitter grammar is a bit of a bottleneck right now, as there are two PRs here to merge, then I have another one coming for another small update, and then we need to update topiary-queries and finally Nickel)

@yannham yannham merged commit da030eb into main Jun 4, 2024
1 check passed
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