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

Embed topiary as "nickel format" #1352

Closed
yannham opened this issue Jun 9, 2023 · 4 comments
Closed

Embed topiary as "nickel format" #1352

yannham opened this issue Jun 9, 2023 · 4 comments
Assignees
Labels

Comments

@yannham
Copy link
Member

yannham commented Jun 9, 2023

Is your feature request related to a problem? Please describe.

Currently, formatting is handled by an external tool (Topiary). This is ok for the time being but requires a bit of logistics: users have to go to the Topiary repository, and follow the installation guide (or use cargo maybe, now that Topiary has been released). The version of Topiary and Nickel aren't necessarily in sync. It could be nice to have formatting built in the Nickel CLI.

Describe the solution you'd like

Instead of relying on Topiary, we could have a native sub-command to format Nickel code, such as nickel format. The plan is simply to embed Topiary inside the Nickel binary (behind a compilation flag, so that if one needs to embed a light interpreter only, they don't have to pay this cost).

Describe alternatives you've considered

  • do nothing, and keep using a separate tool with a separate update process
  • have our own internal formatter, without relying on Topiary. For some context, we tried to do that some time ago without duplicating too much the whole parser infrastructure - it turned out to be non trivial and cumbersome because LALRPOP is just not tailored to formatting (it loses to much source information), so we thought about re-using the tree-sitter grammar instead in some way...which led to the creation of Topiary! So, unless we change our parser infrastructure entirely, we've studied this issue and Topiary seems to be the best and simplest answer as of today
@aspiwack
Copy link
Member

aspiwack commented Jun 9, 2023

@Niols has had on his backburner publishing a crate for Topiary. Which would help quite a bit here, I assume.

@Niols
Copy link
Member

Niols commented Jun 9, 2023

That is correct. It will happen eventually but we're waiting for the current owner of the topiary crate to transfer ownership to us.

@vkleen
Copy link
Contributor

vkleen commented Jun 9, 2023

For a first pass, we can just use a git dependency in Nickel. The crate structure seems to already be split nicely in topiary.

@vkleen
Copy link
Contributor

vkleen commented Jul 18, 2023

This was done in #1371 and #1455.

@vkleen vkleen closed this as completed Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants