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

New: Add 'recommended' configuration #73

Merged
merged 1 commit into from
Dec 17, 2017

Conversation

azz
Copy link
Member

@azz azz commented Dec 16, 2017

Closes #72

Copy link
Member

@not-an-aardvark not-an-aardvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! This generally looks good to me, I just have a few documentation nitpicks.

## Options

> Note: While it is possible to pass options to Prettier via your ESLint configuration file, it is not recommended because editor extensions such as `prettier-atom` and `prettier-vscode` **will** read [`.prettierrc`](https://prettier.io/docs/en/configuration.html), but **won't** read settings from ESLint, which can lead to an inconsistent experience.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a sidenote: I was under the impression that people using eslint-plugin-prettier would just use editor integrations for eslint, because it wouldn't be necessary to use integrations for prettier. Is it common for people to use prettier-atom along with eslint-plugin-prettier?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally use prettier-vscode with eslint-plugin-prettier. I don't use use the vscode-eslint plugin's format on save capabilities as Prettier covers a wider range of languages, and I find it a bit faster.

README.md Outdated
{
"extends": [
"plugin:prettier/recommended"
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it also necessary to use "plugins": ["prettier"] in this config file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, configurations can turn on plugins, including their own.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I had been under the impression that "plugins": ["prettier"] was needed to use plugin:prettier/recommended at all, but after trying it out it seems like you're correct.

Separately, maybe it would be a good idea to update ESLint's plugin-loading so that extends: plugin:foo/bar also implies plugins: ["foo"], so that it wouldn't be necessary to do something like plugins: ["prettier"] in a shareable config within eslint-plugin-prettier itself.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I was surprised that it works the way it does. Not sure if there's a use case for a plugin exporting a configuration without exposing its rules.

README.md Outdated
This plugin works best if you disable all other ESLint rules relating to code formatting, and only enable rules that detect patterns in the AST. (If another active ESLint rule disagrees with `prettier` about how code should be formatted, it will be impossible to avoid lint errors.) You can use [eslint-config-prettier](https://github.com/prettier/eslint-config-prettier) to disable all formatting-related ESLint rules. If your desired formatting does not match the `prettier` output, you should use a different tool such as [prettier-eslint](https://github.com/prettier/prettier-eslint) instead.
## Note

If your desired formatting does not match the `prettier` output, you should use a different tool such as [prettier-eslint](https://github.com/prettier/prettier-eslint) instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: I think it would be better to keep this note next to the comments about eslint-config-prettier. Can you move it to the Recommended Configuration section (maybe under the first paragraph)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -33,7 +33,8 @@ error: Delete `;` (prettier/prettier) at pkg/commons-atom/ActiveEditorRegistry.j
## Installation

```sh
npm install --save-dev prettier eslint-plugin-prettier
npm install --save-dev eslint-plugin-prettier
npm install --save-dev --save-exact prettier
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact installation is recommended for prettier https://prettier.io/docs/en/install.html

@not-an-aardvark not-an-aardvark merged commit e529b60 into prettier:master Dec 17, 2017
@not-an-aardvark
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants