search

tweag / topiary

https://topiary.tweag.io/
MIT License
575 stars 29 forks source link

Catala language support #751

Closed vincent-botbol closed 1 month ago

vincent-botbol commented 1 month ago

First of all, kudos for the great work.

I've been working with the tool to develop a formatter for Catala. Took me a couple of days to get the hang of it (mostly due to the fact that I just discovered tree-sitter queries) but after that it was straightforward! The current state is still very much WIP but it's getting there.

I was not really sure of the workflow to adopt: either make a PR to integrate Catala's query in topiary or fork the tool to simplify development. I went with the former as I felt the first one would be, in the short term, too cumbersome for all of us and we needed a quick development due to an incoming deadline.

Thanks to the recent merge of #716, I did not need to fork topiary anymore which was a huge gain! The Catala's project is implemented in OCaml and heavily relies on the ecosystem's toolchain, i.e., opam. My ideal short-term scenario would be to maintain a repository containing only Catala's queries & config and have a single dependency on (topiary-opam)[https://github.com/tweag/topiary-opam/]. However, topiary-opam does not yet have #716. Hence, I made a new package ((catala-format)[https://github.com/CatalaLang/catala-format]) which really does what topiary-opam do with a slightly different wrapping/installing and an up-to-date topiary submodule.

I wanted to advertise the existence of this and make sure with you that the approach made sense. If you have suggestions/remarks on how this should be done, I'd be grateful!

yannham commented 1 month ago

Hello @vincent-botbol, thanks for the kind words! We're really happy to see Topiary used for Catala. This is precisely what Topiary is for.

I don't know if I'm the best person to judge that, but it seems to me that your current setup is reasonable, although it would be indeed ideal to just maintain the queries and the grammar without needing to fork anything.

I'm not entirely sure what are the plans for topiary-opam, but I suspect the dynamic loading of grammars won't land there at least until it lands in a proper Topiary release first. But knowing that people actually rely the opam package might lead us to make sure it remains up-to-date more pro-actively (for now @Niols is the opam guy, I suppose).

Niols commented 1 month ago

Hi @vincent-botbol, good to see you here. Being a big fan of Catala, I love the idea of it being formatted by Topiary!

I am indeed “the opam guy”, I suppose. Your request sounds entirely reasonable, and there will only be two blockers on my end:

vincent-botbol commented 1 month ago

Thanks for your quick answers.

@yannham

I'm not entirely sure what are the plans for topiary-opam, but I suspect the dynamic loading of grammars won't land there at least until it lands in a proper Topiary release first. This seems indeed very reasonable.

@Niols

topiary-opam actually never included v0.4.0 because the Cargo packaging on Topiary's side changed entirely and broke things. I would need to sit with a Cargo-knowledgeable person for half an hour, I think.

Got it. No pressure at all, the setup I'm currently using does the trick for our current use-case. I'll make sure to clean up things when it lands.

I'll keep the current setup around until the patches land in a release :) Keep up the good work!

Niols commented 1 month ago

The packaging of v0.4.0 is now tracked in https://github.com/tweag/topiary/issues/753. I believe you could open a feature request to have a release that includes dynamic loading!

aspiwack commented 1 month ago

@vincent-botbol is this something that we can talk about/amplify, or are you still keeping the project discrete?

vincent-botbol commented 1 month ago

@aspiwack No troubles on our end. There is no particular discretion needed regarding this project.

Niols commented 2 weeks ago

@vincent-botbol There is a now a release (v0.5.1) that includes dynamic loading, and it has just now landed in opam-repository.

vincent-botbol commented 2 weeks ago

@vincent-botbol There is a now a release (v0.5.1) that includes dynamic loading, and it has just now landed in opam-repository.

Yes, I noticed that. Tested it and it worked perfectly, thanks a lot !