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

feat: support matching dot files #132

Merged
merged 5 commits into from Aug 25, 2022
Merged

feat: support matching dot files #132

merged 5 commits into from Aug 25, 2022

Conversation

u3u
Copy link
Contributor

@u3u u3u commented May 9, 2018

I hope it can format and repair all matching files in my project, but I found that it did not format my configuration file (eg: .eslintrc.js, .postcssrc.js, etc.)

"format": "prettier-eslint \"**/*.{js,json,vue,md}\" --write"

https://github.com/isaacs/node-glob#dots

@codecov-io
Copy link

codecov-io commented May 9, 2018

Codecov Report

Merging #132 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #132   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines         123    123           
  Branches       16     16           
=====================================
  Hits          123    123
Impacted Files Coverage Δ
src/format-files.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8c077b...dfb776d. Read the comment docs.

@Oligrand
Copy link

@kentcdodds @zimme
Any chance we could get this merged? Or at least give us the option to control the dot option of glob? It would be nice to also get files starting with a dot prettified :)

@kylemh
Copy link
Collaborator

kylemh commented Mar 6, 2021

@u3u feel free to resolve merge conflicts

@Oligrand feel free to open a new PR with the same fixes here if above doesn't happen fast enough for you!

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2021

Stale pull request

@JounQin
Copy link
Member

JounQin commented Jul 31, 2022

Dot files are excluded by ESLint and prettier by default AFAIK, maybe this new feature should be behind a flag?

Or maybe we should respect .eslintignore and .prettierignore instead.

@JounQin
Copy link
Member

JounQin commented Aug 25, 2022

I think this should be implemented under a flag, can false by default same as ESLint and prettier.

@changeset-bot
Copy link

changeset-bot bot commented Aug 25, 2022

🦋 Changeset detected

Latest commit: 6415555

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
prettier-eslint-cli Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 25, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

src/format-files.js Outdated Show resolved Hide resolved
@JounQin JounQin changed the title fix: should match dots feat: support matching dots Aug 25, 2022
@JounQin JounQin self-assigned this Aug 25, 2022
@JounQin JounQin changed the title feat: support matching dots feat: support matching dot files Aug 25, 2022
@kylemh
Copy link
Collaborator

kylemh commented Aug 25, 2022

I think a includeDotFiles flag is good; however, you touched on a good point with the ignore files. If we WERE to respect those, how would we do it?

I can see at least 2 ways:

  1. Use each ignore file for their respective runner. So, if ONLY a prettier ignore file exists, the eslint portion of our task will complete as expected and only the prettier portion would adhere to the ignore file.
  2. We merge the ignore files and avoid those files for our entire runner.

@JounQin
Copy link
Member

JounQin commented Aug 25, 2022

Use each ignore file for their respective runner.

I review the source codes we're using this approach now: https://github.com/prettier/prettier-eslint-cli/blob/master/src/format-files.js#L206-L236

So we'll only need to support a includeDotFiles flag.

@JounQin

This comment was marked as resolved.

@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2022

Codecov Report

Merging #132 (6d16127) into master (625a7fe) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 6d16127 differs from pull request most recent head 6415555. Consider uploading reports for the commit 6415555 to get more accurate results

@@            Coverage Diff            @@
##            master      #132   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          166       167    +1     
  Branches        29        29           
=========================================
+ Hits           166       167    +1     
Impacted Files Coverage Δ
src/format-files.js 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@JounQin JounQin merged commit 4561a79 into prettier:master Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants