denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.57k stars 5.33k forks source link

make deno xx.ts --fmt print output to stdout #2090

Closed axetroy closed 5 years ago

axetroy commented 5 years ago

Why print output to stdout?

For better integration by third parties.

Such as the IDE's formatting tool.

ref: https://github.com/denoland/deno_std/pull/332

This is my proposal, I hope it helps.

Proposal 1:

deno fmt xxx.ts # print formatted code to stdout
deno fmt xxx.ts --write # write formatted code to file

~Proposal 2:~

~add a new flags --fmt-stdout for deno~

justjavac commented 5 years ago

gofmt: by default, prints the reformatted sources to stdout, output to files using -w

rustfmt: by default, prints the reformatted sources to file, output to stdout using --emit stdout.

zaynv commented 5 years ago

For most users, won't the use case be to just run it on files to format them? I think it would make more sense to make the default user-friendly by making --write default and then make a flag like you suggested --stdout or something to print it to stdout so editors and such can use it.

Edit: Missed Proposal 2 before I commented, updated my comment accordingly.

kt3k commented 5 years ago

gofmt outputs the formatted contents to stdout, but go fmt subcommand formats the contents and writes into files.

I think deno fmt is modeled after go fmt, so I prefer writing by default, stdout with options.

@ry How do you think?

ry commented 5 years ago

I agree @kt3k

kt3k commented 5 years ago

I'm going to send a PR, implementing the following design:

kt3k commented 5 years ago

I think we need to perform some steps to achieve the above without breaking the latest version of deno.

  1. [x] First, make deno fmt maps to deno run .../std/prettier/main.ts --write, this doesn't make any effect, but it prepares for the change of deno_std, https://github.com/denoland/deno_std/pull/332

  2. [x] Release deno including the above. At this point merging https://github.com/denoland/deno_std/pull/332 doesn't break deno fmt. So we can merge it.

  3. [x] Merge https://github.com/denoland/deno_std/pull/332.

  4. [ ] Implement deno fmt --emit using the above std's change.

ry commented 5 years ago

@kt3k Thanks for handling this! SGTM

axetroy commented 5 years ago

@kt3k

Don't consider adding a flag to make deno fmt output to stdout?

eg deno fmt xxx.ts --emit-stdout or something like this.

kt3k commented 5 years ago

@axetroy Yes, I'm planning it. That's step 4 in the above plans. We need to do that after merging https://github.com/denoland/deno_std/pull/332 (because that needs the function the PR includes.)

--emit-stdout would be ok. I think --emit or --stdout also possible.