lassik / emacs-format-all-the-code

Auto-format source code in many languages with one command
https://melpa.org/#/format-all
MIT License
613 stars 106 forks source link

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

Open venikx opened 2 years ago

venikx commented 2 years ago

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: https://github.com/lassik/emacs-format-all-the-code/blob/6200b91d9151b3177a676d30edd948266292bcc1/format-all.el#L1009 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 commented 2 years ago

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

lassik commented 2 years ago

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

Code here

lassik commented 2 years ago

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

venikx commented 2 years ago

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: https://github.com/lassik/emacs-format-all-the-code/blob/6200b91d9151b3177a676d30edd948266292bcc1/format-all.el#L1009

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 commented 2 years ago

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

venikx commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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: https://github.com/lassik/emacs-format-all-the-code/pull/182

lassik commented 2 years ago

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

venikx commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@lassik Broken change here, 828280eaf3b46112e17746a7d03235570a633425 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