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: add support for glob patterns like prettier (#57) #59

Merged
merged 1 commit into from Jan 17, 2019

Conversation

jantimon
Copy link
Collaborator

@jantimon jantimon commented Jan 15, 2019

This pull request adds the ability to filter files for a given minimatch pattern.
If no pattern is given it will work the same way it did before.

From: https://prettier.io/docs/en/cli.html

prettier --write "{app,__{tests,mocks}__}/**/*.js"

Fixes #57

Copy link
Member

@azz azz left a comment

Choose a reason for hiding this comment

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

Love it. Thanks! Would you be able to document this in README?

@azz
Copy link
Member

azz commented Jan 16, 2019

Would you like to be added as a collaborator on this project?

@jantimon
Copy link
Collaborator Author

Yeah sure 👍

I did three changes:

  • added an entry to the readme
  • switched to minimatch to reduce the amount of dependencies
  • introduced a --pattern prefix because mri will otherwise turn pretty-quick --staged "*.js" into { "staged": "*.js" }.

const path = require('path');

export default pattern => {
if (typeof pattern !== 'string') {
Copy link
Member

Choose a reason for hiding this comment

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

I'd have to double check, but if you do pretty-quick --pattern a --pattern b, does mri make pattern equal ['a', 'b']?

Copy link
Member

Choose a reason for hiding this comment

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

(and if so, should we handle it?)

Copy link
Member

Choose a reason for hiding this comment

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

I think we should either (a) handle multiple patterns or (b) print a warning rather than ignoring multiple patterns. I have no strong feelings one way or the other :P

Copy link
Member

Choose a reason for hiding this comment

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

I also don't mind keeping the multimatch dependency.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’ll take a look and let you know :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It supports now three cases (using multimatch)

  • no patterns
  • 1 pattern (string)
  • n pattern (array of strings)

@jantimon
Copy link
Collaborator Author

jantimon commented Jan 17, 2019

@azz do you think we can merge this now?

@azz azz merged commit 9efd7fd into prettier:master Jan 17, 2019
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