-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add support for prettierIgnore
key in package.json
#12672
Conversation
Anyone? |
any progress 👀? |
src/cli/format.js
Outdated
@@ -227,8 +227,13 @@ function format(context, input, opt) { | |||
} | |||
|
|||
async function createIgnorerFromContextOrDie(context) { | |||
const filepath = context.argv.filepath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this filepath come from? There is no filepath
flag in CLI at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is - as --stdin-filepath
- and it's used here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filepath
only exists when format stdin, when formatting files, it doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See formatFiles
function, it calls createIgnorerFromContextOrDie
on first line. I don't think it will work.
I'd love to see this land. It would allow us to have pretterignore settings shared across projects. As we can already share settings, but not ignored files (https://prettier.io/docs/en/configuration.html#sharing-configurations) Is there anything we could contribute to accelerate this? |
I would suggest adding this as an option within the configuration file (i.e. |
I don't think sharing ignored files would be a good practice, as ignored files tend to vary more based on a project's tooling usage than its style guide, and we wouldn't want to couple something unrelated to style to a config. |
Yes, they do. |
That's a different feature for a different PR. |
src/common/create-ignorer.js
Outdated
const ignoreContent = !ignorePath | ||
? null | ||
: getFileContentOrNull.sync(path.resolve(ignorePath)); | ||
createIgnorer.sync = function (filePath, ignorePath, withNodeModules) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change base to next
branch, so we don't need support sync version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
It might not be a bad idea to implement this feature directly in the So when we want to share the settings we have all the options inside one file. This is exactly what For example, I can specify in {
"name": "package.json",
"type": "module",
"prettier": "./.config/prettier.config.cjs",
"eslintConfig": {
"extends": "./.config/eslint.config.cjs"
}
} and in one file I have all settings, including // .config/eslint.config.js
module.exports = {
root: true,
extends: [],
plugins: [],
ignorePatterns: [
'.DS_Store',
'node_modules'
// ...
]
} This would be a huge win to put // .config/prettier.config.js
module.exports = {
singleQuote: true,
trailingComma: 'none',
plugins: [],
ignorePatterns: [
'.DS_Store',
'node_modules'
// ...
]
} What do you think? |
@mataha would you be interested to continue this PR? @ivodolenc's suggestion to use |
I second @ivodolenc's suggestion: #12672 (comment) It would be really great to be able to specify |
Note, as this appears to have stalled out, I've started working on adding the options to the configuration. As a first step, I've added command line arguments to echo ESLint - as they were quite easy to do, and I've just posted that in #14223 |
Is there anything I can do to help get this over the finish line? In a world where we already have a ton of config files in the root of our projects, I'm struggling to understand why we can't specify patterns to ignore in our prettier config file and we're forced to create a new file just for that. |
As discussed elsewhere, I think it would be much better to add an option in the I tried doing this previously but hit some issues, and despite asking never got any feedback from the developers, even when pinging the devs directly, so I gave up. Feel free to take my work and try moving it forward though, maybe you'll have more success. |
This is a rather dumb implementation (for now). Implements #3460 Signed-off-by: mataha <mataha@users.noreply.github.com>
With a sample `package.json`. Signed-off-by: mataha <mataha@users.noreply.github.com>
Not sure if more are necessary? Signed-off-by: mataha <mataha@users.noreply.github.com>
Signed-off-by: mataha <mataha@users.noreply.github.com>
Signed-off-by: mataha <mataha@users.noreply.github.com>
Signed-off-by: mataha <mataha@users.noreply.github.com>
Signed-off-by: mataha <mataha@users.noreply.github.com>
Signed-off-by: mataha <mataha@users.noreply.github.com>
I'm in a position in which I can commit some time to this again. I'd like to clarify some things beforehand though.
Again, I believe that's a different feature for a different PR. The core tenet of this is that one can share That said, the main pain point of this PR continues to be how Prettier should behave upon encountering a
I'll have to rewrite this as several things have changed since then (e.g. ignore facility has moved to |
Can't run tests at all due to jestjs/jest#7914. |
Description
Most of what I've wanted to write about had already been discussed in #3460; to keep it brief:
package.json
is used more and more to store configuration for various development tools (including their gitignore-like files, if any) in lieu of dotfiles. Prettier provides a way to do that for its options, but not for the list of files to ignore itself.This PR tries to amend that. Implementation is similar to ESLint - via
prettierIgnore
key.Checklist
docs/
directory).changelog_unreleased/*/XXXX.md
file followingchangelog_unreleased/TEMPLATE.md
.✨Try the playground for this PR✨