search

astrale-sharp / typstfmt

Apache License 2.0
253 stars 25 forks source link

Merging efforts #56

Closed jeffa5 closed 1 year ago

jeffa5 commented 1 year ago

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: https://github.com/typst/typst/issues/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 commented 1 year ago

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 commented 1 year ago

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

Sorry, I meant syntax nodes

astrale-sharp commented 1 year ago

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

astrale-sharp commented 1 year ago

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

RossSmyth commented 7 months ago

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: https://github.com/rust-lang/rust-analyzer/issues/1665 A previous WIP impl: https://github.com/rust-lang/rust-analyzer/pull/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 commented 7 months ago

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