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

Skip globbing filenames with node-glob when the filename is not a glob #1307

Merged
merged 2 commits into from Apr 17, 2017

Conversation

hawkrives
Copy link
Contributor

Closes #1246

@vjeux:

For this issue, I can see two things:

  1. Use fs.readFileSync instead of going through glob is there is no special patterns. If you give the actual name of a file, we should throw if it's not there.
    […]

It turns out thatnode-glob has a handy function .hasMagic, which tells you if a given string has any globbable patterns in it. All I had to do was call it when we iterate over the filenames.

I tested this manually; I'm not sure how to add tests for the prettier cli.

# before
$ stat non-existent-file.js
stat: cannot stat 'non-existent-file.js': No such file or directory
$ prettier non-existent-file.js; echo $?
0
# after
$ stat non-existent-file.js
stat: cannot stat 'non-existent-file.js': No such file or directory
$ node bin/prettier.js non-existent-file.js; echo $?

Unable to read file: non-existent-file.js
Error: ENOENT: no such file or directory, open 'non-existent-file.js'
2

@hawkrives hawkrives changed the title add a check to skip node-glob if the filename is not a glob Skip globbing filenames with node-glob when the filename is not a glob Apr 16, 2017
bin/prettier.js Outdated
@@ -290,6 +290,10 @@ if (stdin) {

function eachFilename(patterns, callback) {
patterns.forEach(pattern => {
if (!glob.hasMagic(pattern)) {
callback(pattern)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should put a return after so it doesn't also do the glob

@vjeux
Copy link
Contributor

vjeux commented Apr 16, 2017

Thanks! Looks great :) If you fix the comment I'll merge it.

@hawkrives
Copy link
Contributor Author

Done! And I added a semicolon for consistency.

@m1ck2
Copy link

m1ck2 commented Apr 17, 2017

You do realise that you's are swapping names and email addresses around in google and different again here.

@vjeux vjeux merged commit 3e5b123 into prettier:master Apr 17, 2017
@hawkrives
Copy link
Contributor Author

@m1ck2 I'm sorry? Both of those commits have the same name and email, do they not?

@vjeux
Copy link
Contributor

vjeux commented Apr 18, 2017

@hawkrives: I think it's a bot :( I've seen a lot of random messages like this that barely make sense on many repos across github.

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 20, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
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

3 participants