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

Add --noglob cli option for processing exact the passed files #6492

Closed
wants to merge 4 commits into from
Closed

Add --noglob cli option for processing exact the passed files #6492

wants to merge 4 commits into from

Conversation

AzazKamaz
Copy link

@AzazKamaz AzazKamaz commented Sep 17, 2019

I faced problem with configuring git hooks with lint-staged on sapper framework. lint-staged starts prettier and passes changed files with arguments, but in sapper there are routes with name [name].svelte, and prettier thinks that is is a regexp. So I have added parameter (--noglob) that disables any logic on filenames and just keeps it as you write, so it will process exactly what you have passed

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to the CHANGELOG.unreleased.md file following the template.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@AzazKamaz
Copy link
Author

With this option, prettier will work like eslint. There are no such logic with glob)

@kachkaev
Copy link
Member

This CLI option should be also helpful to those using dynamic routing in Next.js https://github.com/zeit/next.js#dynamic-routing. The PR resolves #6344.

Instead of tweaking Prettier, the solution might actually come from lint-staged/lint-staged#698 (see discussion in lint-staged/lint-staged#676). The benefit in that approach is that we don't need any extra CLI flags in Prettier, which is great.

If this PR gets accepted, it might be worth updating https://prettier.io/docs/en/precommit.html as well.

@AzazKamaz
Copy link
Author

Instead of tweaking Prettier, the solution might actually come from okonet/lint-staged#698 (see discussion in okonet/lint-staged#676). The benefit in that approach is that we don't need any extra CLI flags in Prettier, which is great.

If okonet/lint-staged will provide escaped filenames, than eslint/eslint will not find them, because there are no globbing on [] and it will try to find file like \[test\].js

@alexerhardt
Copy link

I'm using Next.js with a Prettier file watcher and this feature would be a lifesaver for me. As it stands dynamic routes throw errors on save, and I can't see an easy solution to this problem. Thanks for the effort @AzazKamaz - hope it gets accepted.

@lydell
Copy link
Member

lydell commented Sep 29, 2019

Random thought: Maybe it would even be nice if Prettier didn’t support [abc] syntax at all. I’ve never used it. My globs are all {a,b,c}, *.js and src/**/*.css. ¯\_(ツ)_/¯

@alexander-akait
Copy link
Member

@lydell
Copy link
Member

lydell commented Sep 30, 2019

@evilebottnawi Ah – first treat as literal file name, and if no such file exists then treat as glob? Sounds good to me.

(We could potentially end up in the reverse situation – a file on disk prevents someone’s glob to be treated as a glob, but that should be workaround-able by adjusting the glob pattern.)

@lydell
Copy link
Member

lydell commented Nov 9, 2019

This is now tracked in #6854.

@lydell lydell closed this Nov 9, 2019
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Feb 8, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants