Don't default parser to babylon by duailibe · Pull Request #4528 · prettier/prettier

duailibe · 2018-05-23T16:23:13Z

This takes a different approach from #4426 to make it more explicit a parser wasn't specified or inferred (returns normalized opts as null and format() bails formatting).

lydell · 2018-05-23T16:28:42Z

src/main/options.js

+  if (!rawOptions.parser) {
+    const logger = (opts && opts.logger) || console;
+    if (!rawOptions.filepath) {
+      logger.warn("Skipping file with unknown filepath");


What is an unknown filepath?

@lydell Use stdin without --stdin-filepath or prettier.format without { filepath }

At first I decided to throw an error but I figured that could be a little too harsh

I see. I wanted to come up with a better message, but it's hard.

Really like the updated message, good job!

lydell · 2018-05-23T16:48:53Z

I fetched your branch and played around with it a little, and it seems nice.

Minor logging glitch:

❯ ./bin/prettier.js LICENSE index.js --write
LICENSESkipping file with unknown extension: LICENSE
LICENSE 2ms
index.js 59ms

Question about stdout:

❯ ./bin/prettier.js LICENSE

Currently the above print LICENSE as-is to stdout. Is it better to not print it to stdout at all? Similar: What should prettier.format("foo") return?

suchipi · 2018-05-23T16:55:49Z

src/main/core-options.js

    category: CATEGORY_GLOBAL,
    type: "choice",
-    default: "babylon",
+    default: undefined,


For the support info, you need to specify that the default varies across versions; see #4426 (comment)

suchipi

LGTM, aside from the logging issue @lydell mentioned

duailibe · 2018-05-23T17:04:54Z

Currently the above print LICENSE as-is to stdout. Is it better to not print it to stdout at all? Similar: What should prettier.format("foo") return?

Good questions, I'm not sure. I'm comfortable with throwing an error in the latter, but I'm not sure what we should do about prettier LICENSE

duailibe · 2018-05-23T17:44:40Z

So I tried to improve the logging experience a bit but there are too much cases to cover 😅

This code is pretty complex, I'd appreciate more testing

duailibe · 2018-05-23T18:28:19Z

I think I covered most use cases in infer-parser.js; should be easier to reason about the behavior now

duailibe · 2018-05-23T18:33:07Z

tests_integration/__tests__/infer-parser.js

+  });
+});
+
+describe("--write and --list-different with unknown path and no parser", () => {


I have no idea what this means (how it should behave).. we really need to revamp the CLI

lydell

This is so good! 😍

suchipi · 2018-05-23T20:09:47Z

🎉

**What kind of change does this PR introduce?** Bugfix **Did you add tests for your changes?** No **If relevant, did you update the documentation?** No **Summary** ![image](https://user-images.githubusercontent.com/5961873/42949683-ef845284-8b8f-11e8-9f3a-d8d4c673a542.png) Prettier shows warning currently that filePath and parser aren't specified. It would throw errors in future major versions. [#4828](prettier/prettier#4528) This PR cleans the warning shown in the above image.   **Does this PR introduce a breaking change?** No **Other information** No

Don't default parser to babylon

bae5f8a

lydell reviewed May 23, 2018

View reviewed changes

suchipi reviewed May 23, 2018

View reviewed changes

suchipi mentioned this pull request May 23, 2018

Don't default parser to babylon #4426

Closed

Different default values according to versions

2ad9a1f

suchipi approved these changes May 23, 2018

View reviewed changes

Fix error logging

ccc3fdd

duailibe added 2 commits May 23, 2018 14:50

Revert test

60e2785

Add tests specifically for parser inference

a316432

Add another test case

9f571a9

duailibe commented May 23, 2018

View reviewed changes

duailibe changed the title ~~Don't default parser to babylon (2)~~ Don't default parser to babylon May 23, 2018

duailibe added 2 commits May 23, 2018 15:58

Add API test

ed0429a

Remove dead code

6fc9127

lydell approved these changes May 23, 2018

View reviewed changes

duailibe merged commit 5c6e080 into prettier:master May 23, 2018

duailibe deleted the default-parser branch May 23, 2018 19:55

This was referenced May 24, 2018

Fix playground #4541

Merged

Update docs for default parser #4557

Merged

This was referenced May 28, 2018

[v13] fix: Explicitly set the prettier parser to 'babylon' vuejs/vue-loader#1322

Closed

[v14] fix: Explicitly set the prettier parser to 'babylon' vuejs/vue-loader#1323

Merged

ikatyang mentioned this pull request May 30, 2018

fix(cli): regression for --debug-check + --list-different #4600

Merged

uxuf mentioned this pull request Jun 4, 2018

1.13.4 Babylon does not correctly parse TypeScript code #4633

Closed

duailibe mentioned this pull request Jun 15, 2018

--list-different twice as slow as --write on large codebase #4345

Closed

ikatyang mentioned this pull request Jun 17, 2018

Noooooo my .git is dead! / Ignore dotfiles? #2882

Closed

johanhenrikssn mentioned this pull request Jul 13, 2018

Update prettier facebook/react#13205

Merged

dhruvdutt mentioned this pull request Jul 19, 2018

fix(prettier): add parser, filePath webpack/webpack-cli#553

Merged

lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Aug 21, 2018

lock bot locked as resolved and limited conversation to collaborators Aug 21, 2018

Uh oh!

Conversation

duailibe commented May 23, 2018

Uh oh!

lydell May 23, 2018

Choose a reason for hiding this comment

Uh oh!

duailibe May 23, 2018

Choose a reason for hiding this comment

Uh oh!

duailibe May 23, 2018

Choose a reason for hiding this comment

Uh oh!

lydell May 23, 2018

Choose a reason for hiding this comment

Uh oh!

lydell May 23, 2018

Choose a reason for hiding this comment

Uh oh!

lydell commented May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suchipi May 23, 2018

Choose a reason for hiding this comment

Uh oh!

suchipi left a comment

Choose a reason for hiding this comment

Uh oh!

duailibe commented May 23, 2018

Uh oh!

duailibe commented May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

duailibe commented May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

duailibe May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lydell left a comment

Choose a reason for hiding this comment

Uh oh!

suchipi commented May 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lydell commented May 23, 2018 •

edited

Loading

duailibe commented May 23, 2018 •

edited

Loading

duailibe commented May 23, 2018 •

edited

Loading

duailibe May 23, 2018 •

edited

Loading