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

[WIP] Use dprint for internal js/ts formating #4507

Closed
wants to merge 7 commits into from

Conversation

keroxp
Copy link
Contributor

@keroxp keroxp commented Mar 28, 2020


  • Dprint - Use cargo run fmt with latest deno build
  • Code adaption for dprint bug
    • All error syntaxes (for dprint) have been fixed.
  • Excluded files -- currently there's no way to exclude specified files for deno fmt
    • .prettierignore doesn't work
  • Build for per formatting - Because cargo run will re-build entire project...
  • Huge stdout
    • cargo run shows all expanded glob arguments...

@bartlomieju
Copy link
Member

Hey @keroxp I'm very glad you're tackled this issue!

Just yesterday @dsherret merged PR to add Rust shell to dprint (dprint/dprint#162), and it's now available on https://crates.io/crates/dprint.
That means you should be able to cargo install dprint just like for rustfmt. It's a simple shell that works almost the same as deno fmt, the only difference is that dprint doesn't walk directory tree to find files, but requires to pass all of them as CLI arguments.

Keep in mind that there are still some quirks for Deno codebase when using dprint.

@kitsonk
Copy link
Contributor

kitsonk commented Mar 28, 2020

Also, although this is great, we should merge #4498 first as it includes a move to ES private fields which was blocked because of a lack of Prettier support. I "could" unpick it, but it is complex.

I also agree that we should use the standalone binary, instead of another version of Deno.

@bartlomieju
Copy link
Member

Also, although this is great, we should merge #4498 first as it includes a move to ES private fields which was blocked because of a lack of Prettier support. I "could" unpick it, but it is complex.

I also agree that we should use the standalone binary, instead of another version of Deno.

@kitsonk, yeah agreed #4498 should be landed first

@@ -72,7 +72,7 @@ export class ListenerImpl implements Listener {
async *[Symbol.asyncIterator](): AsyncIterator<Conn> {
while (true) {
try {
yield await this.accept();
yield (await this.accept());
Copy link
Member

Choose a reason for hiding this comment

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

This was fixed in swc so is not necessary anymore.

@dsherret
Copy link
Member

Before you actually run it on the code, could we wait until this milestone is resolved in dprint? https://github.com/dsherret/dprint/milestone/2

It's way less problems than before, but still a bit of work to do. Might be two more weeks or so :)

@keroxp
Copy link
Contributor Author

keroxp commented Mar 29, 2020

@dsherret Yes, this may be too early adaption to dprint. I'll re-try after that.

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.

Migrate from prettier to dprint for internal formatting
4 participants