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

Possibility of removing the prettier parser arguments, or add the ability to configure your own #175

Open
venikx opened this issue Feb 17, 2022 · 12 comments

Comments

@venikx
Copy link

venikx commented Feb 17, 2022

I'm currently running into an issue with prettier formatting the package.json using the json5 parser, while Emacs formats it with the json parser. The CLI arguments take precedence over the .prettierrc overrides, so even when I try adding json5 as the override for all json files, it doesn't respect due the CLI parser arguments.

Thus, I'm wondering why the prettier formatter depends on the parser arguments, see:

"--parser" (let ((pair (assoc language

Is there some special cases I'm currently unaware of?

According to the prettier documentation, choosing the correct parser is supposed to be handled by prettier itself. If you really need to change some parser, there's possibility to override using the .prettierrc, for example:

{
  "overrides": [
    {
      "files": ["*.json"],
      "options": {
        "parser": "json"
      }
    }
  ]
}
@lassik
Copy link
Owner

lassik commented Mar 21, 2022

Commit 6bcd9a5 should fix JSON5 for prettier. Should be in MELPA in a couple hours. Does it work?

@lassik
Copy link
Owner

lassik commented Mar 21, 2022

JSON5 is detected when you use json-mode and the filename extension is .json5.

Code here

@lassik
Copy link
Owner

lassik commented Mar 21, 2022

If there are other JSON5 modes for Emacs (e.g. in web-mode), please let me know and I'll add them.

@venikx
Copy link
Author

venikx commented Mar 22, 2022

The main issue is that prettier by default formats .json as json5, unless you change that in the parser options I mentioned above. Currently, the workaround is to add the snippet above in your .prettierrc so it reflects how format-all overrides the parser on the command line, see:

"--parser" (let ((pair (assoc language

In essence, if you don't add the workaround it means that for json files, running prettier in the command line and running prettier in the editor gives a slightly different result. Though, it's not a major problem, ideally this line should not be needed as you can configure the parser options with prettier itself. I'm just unsure on the impact, because it does mean if people haven't properly configured prettier parser options in the .prettierrc their formatting would suddenly be different.

@lassik
Copy link
Owner

lassik commented Mar 24, 2022

Does prettier use json5 as the default parser for both .json and .json5 files?

@venikx
Copy link
Author

venikx commented Mar 29, 2022

From what I tested it seems to be yes, but I think that's beside my point. We shouldn't be overriding the parser options to begin with as you configure the parser options using the .prettierrc file.

@lassik
Copy link
Owner

lassik commented Apr 1, 2022

Good point. But format-all should still choose a formatter when there's no rc file, or the rc file does not specify a formatter.

Would --config-precedence file-override solve the problem?

https://prettier.io/docs/en/cli.html#--config-precedence

--config-precedence
Defines how config file should be evaluated in combination of CLI options.

cli-override (default)

CLI options take precedence over config file

file-override

Config file take precedence over CLI options

prefer-file

If a config file is found will evaluate it and ignore other CLI options. If no config file is found, CLI options will evaluate as normal.

This option adds support to editor integrations where users define their default configuration but want to respect project specific configuration.

@venikx
Copy link
Author

venikx commented Apr 1, 2022

I had something written here, that I don't agree with anymore.

EDIT: The thing is that prettier already guesses the correct parser for you, it's not something you usually configure yourself. So even if you don't have an rc file, prettier will still work and correctly format all the files it knows how to format (if you run them through prettier).

Basically, I'm suggesting this: #182

lassik added a commit that referenced this issue Apr 4, 2022
Let `prettier` itself select the right parser for named files. It can
look at the file name extension to figure out which one to pick.
@lassik
Copy link
Owner

lassik commented Apr 4, 2022

Please try out the fix I just pushed to the lassik-prettier branch.

@venikx
Copy link
Author

venikx commented May 10, 2022

Please try out the fix I just pushed to the lassik-prettier branch.

My apologies for the extreme late reply. I finally got around to testing your branch. Looks like it's working fine! :)
I'll close my PR as well, as it's deprecated by your suggestion.

lassik added a commit that referenced this issue May 10, 2022
Let `prettier` itself select the right parser for named files. It can
look at the file name extension to figure out which one to pick.
@lassik
Copy link
Owner

lassik commented May 10, 2022

No problem. Glad to hear it works. I merged it to master. Let me know if any problems come up.

For reference, that's commit 828280e.

@liuyinz
Copy link

liuyinz commented Jun 13, 2022

@lassik Broken change here, 828280e cause another problem.
I have yaml configuration file named ".yamllint", it's major-mode is yaml-mode, but still get no parser error, because prettier couldn't select proper parser automatically, and there is no cli arguments -parser added by force.

I agree we should let prettier select parser automatically, but if it fails, we need to add arguments by langugae-id

lassik added a commit that referenced this issue Jul 1, 2022
Alternative impelementation of PR #195

See also:

- Issue #175
- PR #190, #182
lassik added a commit that referenced this issue Jul 1, 2022
Alternative impelementation of PR #195

See also:

- Issue #175
- PR #190, #182
lassik added a commit that referenced this issue Jul 1, 2022
Alternative impelementation of PR #195

See also:

- Issue #175
- PR #190, #182
lassik added a commit that referenced this issue Jul 18, 2022
Alternative impelementation of PR #195

See also:

- Issue #175
- PR #190, #182
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

No branches or pull requests

3 participants