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

feat(prettier): output to stdout instead of write file by default unless specified --write flag #332

Merged
merged 14 commits into from May 21, 2019

Conversation

3 participants
@axetroy
Copy link
Contributor

commented Apr 10, 2019

Why?

  1. In order to stay consistent with prettier cli
  2. for deno fmt example.ts to print formatted code in stdout
  3. better integrated with IDE(eg. vscode/webstorm/atom...)

before: without --write

deno run prettier/main.ts script1.ts --allow-read --allow-write

after: with --write

deno run prettier/main.ts script1.ts --allow-read --allow-write --write

This PR bring BREAKING CHANGE: Prettier now output to stdout instead of overwriting the file by default. unless specified --write flag

When should be output to stdout?

  1. without --write/--check flag
  2. when source content not equal to formatted content

axetroy added some commits Apr 10, 2019

@kt3k

kt3k approved these changes Apr 12, 2019

@kt3k

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

I think this feature is nice to have.

We need to be careful with the timing of merging of this PR because this will break deno fmt temporarily.

@axetroy axetroy changed the title feat: add --write flags to enable write file. default print to stdout Prettier: add --write flags to enable write file. default print to stdout Apr 12, 2019

@axetroy axetroy changed the title Prettier: add --write flags to enable write file. default print to stdout Prettier: output to stdout instead of write file by default unless specified --write flag Apr 12, 2019

@axetroy

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2019

@ry it's ready for review.

@ry

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2019

@axetroy It LGTM. I'm a bit concerned that this will break "deno fmt" (we should be using a tagged link to std/prettier and not master link).

@axetroy

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

@ry yes. it will break the demo fmt

And I think about again that when should be output to stdout?

When should be output to stdout?

  1. without --write/--check flag
  2. when source content not equal to formatted content

formatted code should always be printed to stdout even source content equal to formatted content. How about it?

@axetroy axetroy changed the title Prettier: output to stdout instead of write file by default unless specified --write flag feat(prettier): output to stdout instead of write file by default unless specified --write flag May 1, 2019

axetroy added some commits May 16, 2019

@kt3k

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

@ry I think it's now safe to merge this PR. I checked deno fmt now maps to run prettier/main.ts <files> --write.

$ deno version
deno: 0.6.0
v8: 7.6.53
typescript: 3.4.1
$ deno fmt -D . | grep prettier/main | grep JS
DEBUG JS - args [ "https://deno.land/std/prettier/main.ts", ".", "--write" ]
Show resolved Hide resolved prettier/main.ts Outdated
@kt3k

kt3k approved these changes May 21, 2019

Copy link
Contributor

left a comment

@axetroy Thank you for updating! LGTM

@axetroy axetroy changed the title feat(prettier): output to stdout instead of write file by default unless specified --write flag [WIP] feat(prettier): output to stdout instead of write file by default unless specified --write flag May 21, 2019

@axetroy

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

waiting for #424

@ry

ry approved these changes May 21, 2019

Copy link
Contributor

left a comment

LGTM

@ry ry merged commit 434007b into denoland:master May 21, 2019

5 checks passed

denoland.deno_std Build #20190521.12 succeeded
Details
denoland.deno_std (Linux) Linux succeeded
Details
denoland.deno_std (Mac) Mac succeeded
Details
denoland.deno_std (Windows) Windows succeeded
Details
license/cla Contributor License Agreement is signed.
Details
@ry

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

Thanks @axetroy, @kt3k!

@axetroy axetroy changed the title [WIP] feat(prettier): output to stdout instead of write file by default unless specified --write flag feat(prettier): output to stdout instead of write file by default unless specified --write flag May 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.