emacs-format-all-the-code icon indicating copy to clipboard operation
emacs-format-all-the-code copied to clipboard

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

Open venikx opened this issue 3 years ago • 12 comments
trafficstars

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"
      }
    }
  ]
}

venikx avatar Feb 17 '22 14:02 venikx

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

lassik avatar Mar 21 '22 06:03 lassik

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

Code here

lassik avatar Mar 21 '22 06:03 lassik

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

lassik avatar Mar 21 '22 06:03 lassik

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.

venikx avatar Mar 22 '22 13:03 venikx

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

lassik avatar Mar 24 '22 08:03 lassik

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.

venikx avatar Mar 29 '22 09:03 venikx

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.

lassik avatar Apr 01 '22 17:04 lassik

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

venikx avatar Apr 01 '22 20:04 venikx

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

lassik avatar Apr 04 '22 17:04 lassik

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.

venikx avatar May 10 '22 20:05 venikx

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.

lassik avatar May 10 '22 21:05 lassik

@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

eki3z avatar Jun 13 '22 03:06 eki3z