Skip to content
This repository has been archived by the owner on Jun 8, 2024. It is now read-only.

Merging efforts #56

Closed
jeffa5 opened this issue Jul 17, 2023 · 6 comments
Closed

Merging efforts #56

jeffa5 opened this issue Jul 17, 2023 · 6 comments

Comments

@jeffa5
Copy link

jeffa5 commented Jul 17, 2023

Before the rewrite here I forked and did a rewrite to be based on the AST myself: https://github.com/jeffa5/typstfmt

I wonder if there is anything that would be useful to merge between these?

I was focusing on having it be configurable to the point that it can print out the original text too if all configuration is off.

Also, I set up the Typst packages repo as a submodule for testing against easily which showed a couple of issues in the typst parser: typst/typst#1690

I think ultimately it would be nice to have the formatter work on Typst's AST (https://github.com/typst/typst/blob/9b1a2b41f0bb2d62106e029a5a0174dcf07ae0d2/crates/typst/src/syntax/ast.rs#L18) which makes things simpler than going through AST nodes and should match the current typst version better. The main blocker for this is that it doesn't include comments so those get stripped out.

@astrale-sharp
Copy link
Owner

Hmm interesting approach!

I don't think it would be easy to merge from one to the other, i care about configurability as well but it's not an immediate priority (with all "features" i want it to work, be pretty, and lossless)

I'm not sure I understand the difference between working on the AST and the AST nodes?

@jeffa5
Copy link
Author

jeffa5 commented Jul 17, 2023

I'm not sure I understand the difference between working on the AST and the AST nodes?

Sorry, I meant syntax nodes

@astrale-sharp
Copy link
Owner

Oh then currently we're already working with the AST! :)

@astrale-sharp
Copy link
Owner

Im gonna close this for now, might take a look at your work from time to time either way! ;)

@RossSmyth
Copy link

I think you misunderstood what he meant. This tool does it similar to what rustfmt does and directly manipulates and outputs strings. Which is alright, but in the long-run becomes a mess. Rustfmt has been hitting pain points because of this for a long time, and has been looking for people to switch away from the stringy API. Luckily Typst uses a CST so it avoids some of the pitfalls that Rustfmt has, but not all. The Node -> String API is tempting, but creating a new tree is more powerful and in general will lead to better UX and less of a chance to make mistakes. Rust has a great type system so I would recommend using it.

Typst has a nice CST interface that this tool uses, but surprisingly not for formatting even though that is a use suited for CSTs. It makes it much easier to make DSL/API for formatting

I would urge you to read this issue: rust-lang/rust-analyzer#1665
A previous WIP impl: rust-lang/rust-analyzer#1678
A real-world impl of this concept: https://github.com/nix-community/nixpkgs-fmt/

I would at least look in to it and think about it.

@astrale-sharp
Copy link
Owner

Hey there :) that's indeed why I want to make a big refactor, thanks for all the links!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants