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

Dynamic loading of grammars #716

Merged
merged 18 commits into from
Sep 10, 2024
Merged

Conversation

ErinvanderVeen
Copy link
Collaborator

@ErinvanderVeen ErinvanderVeen commented Jul 30, 2024

Dynamic loading

Issue: #4

Description

The goal of this PR is to remove our dependencies on the tree-sitter grammar crates, and fetch and compile the grammars as necessary. Ideally, Topiary should not know about any language at all, and fetch all information from the configuration file.

Checklist

Checklist before merging:

  • CHANGELOG.md updated
  • README.md up-to-date

@catskul
Copy link

catskul commented Aug 5, 2024

This is interesting. Any chance this might be provisionally usable at the moment?

@ErinvanderVeen
Copy link
Collaborator Author

This is interesting. Any chance this might be provisionally usable at the moment?

It works on Linux right now (including NixOS), but I've only really tested the happy paths. So be careful!

@ErinvanderVeen ErinvanderVeen force-pushed the 4-dynamic-loading-of-language-support branch 4 times, most recently from 3f55258 to c131ff5 Compare August 16, 2024 13:43
@catskul
Copy link

catskul commented Aug 16, 2024

Passing! Hooray! : )

Nice work.

@ErinvanderVeen ErinvanderVeen force-pushed the 4-dynamic-loading-of-language-support branch 2 times, most recently from 32d8d72 to b806a53 Compare August 21, 2024 09:12
@ErinvanderVeen ErinvanderVeen changed the title 4 dynamic loading of language support Dynamic loading of grammars Aug 21, 2024
@ErinvanderVeen ErinvanderVeen force-pushed the 4-dynamic-loading-of-language-support branch 4 times, most recently from 6885233 to 8d9ea3f Compare August 22, 2024 17:18
@ErinvanderVeen ErinvanderVeen marked this pull request as ready for review August 23, 2024 11:09
Copy link
Member

@Xophmeister Xophmeister left a comment

Choose a reason for hiding this comment

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

Nice 🎉

I don't claim to be an expert on this, so it might be worth getting a second opinion, but I could follow it well enough. Either way, perhaps adding some comments into the language building code would make it more maintainable.

topiary-config/src/language.rs Show resolved Hide resolved
topiary-config/src/language.rs Show resolved Hide resolved
@ErinvanderVeen ErinvanderVeen force-pushed the 4-dynamic-loading-of-language-support branch from 487b142 to 925533f Compare September 10, 2024 14:52
@ErinvanderVeen ErinvanderVeen merged commit a495ecd into main Sep 10, 2024
9 checks passed
@ErinvanderVeen ErinvanderVeen deleted the 4-dynamic-loading-of-language-support branch September 10, 2024 16:56
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.

3 participants