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

Better support for file path wildcards #362

Closed
avocadowastaken opened this issue Jan 21, 2017 · 4 comments
Closed

Better support for file path wildcards #362

avocadowastaken opened this issue Jan 21, 2017 · 4 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program

Comments

@avocadowastaken
Copy link
Contributor

avocadowastaken commented Jan 21, 2017

Lets say we have this structure

  • src
    • foo
      • bar
    • baz

Using macOS 10.12.2 and node v7.4.0 - this command prettier --write src/**/*.js will ignore content inside of bar directory.

After some research I found that process.argv works like that and there are no workaround. (At least I didn't find any).

And I made my own tool using glob and it worked how I expected.

const fs = require('fs');
const glob = require('glob');
const prettier = require('prettier');
const minimist = require('minimist');

const argv = minimist(process.argv.slice(2));

const files = argv._.length ? argv._ : glob.sync('+(src|test)/**/*.js');

files.forEach((filePath) => {
  fs.readFile(filePath, 'utf8', (readError, source) => {
    console.log(filePath)

    if (readError) {
      console.error(readError);
    } else {
      try {
        const result = prettier.format(source);

        fs.writeFile(filePath, result, (writeError) => {
          if (writeError) {
            console.error(writeError);
          }
        });
      } catch (formatError) {
        console.error(formatError);
      }
    }
  });
});

I know that prettier will Resist adding configuration (#40)
And idea about .prettierrc was already declined (#154)

But what about adding .prettierrc to add current available options only ?

{
  "source": [
    "src/**/*.js",
    "test/**/*.js"
  ],
  "parser": "babylon"
}

Or add --source=src/**/*.js parameter to define which files should be prettified.

And use tools like glob to list all available files from source wildcards.

I will be happy to add PR if you're agree with this proposal.

@lydell
Copy link
Member

lydell commented Jan 21, 2017

Regarding globs – it all depends on what your shell supports. In particular, if it supports the ** glob or not.

If a shell supports * but not **, what happens is that you've simply put two *s after each other, which matches exactly the same things as a single *. ("Match zero or more of anything, followed by zero or more of anything (except /s)".)

In bash, you must make sure to enable the "globstar" option (and make sure to run a version of bash that has that option).

# Here's my bash version, which I know has the "globstar" option.
$ bash --version
GNU bash, version 4.3.46(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

# Enter bash (just to show that I'm not using some other shell).
$ bash

# Make sure that the "globstar" option is disabled.
$ shopt -u globstar

# Try a `**` glob.
$ echo src/**/*.js
src/baz/baz.js src/foo/foo.js

# Enable the "globstar" option.
$ shopt -s globstar

# Try a `**` glob again. Note how src/foo/bar/bar.js and src/src.js are found now.
$ echo src/**/*.js
src/baz/baz.js src/foo/bar/bar.js src/foo/foo.js src/src.js

# As far as I know, the "sh" shell does not support the `**` glob (at least not
# by default on my system):
$ sh
$ echo src/**/*.js
src/baz/baz.js src/foo/foo.js
$ echo src/*/*.js
src/baz/baz.js src/foo/foo.js

Since there are so many shells and so many versions, it's difficult to get globbing right. To make things worse, the default shell in many operating system doesn't support the ** glob (by default). This is especially a problem in npm scripts:

{
  "scripts": {
    "prettier": "prettier src/**/*.js"
  }
}

The above is not a good cross-platform solution. Therefore, it is common among Node.js CLI tools to incorprate their own globbing support, allowing to write the above example as:

{
  "scripts": {
    "prettier": "prettier 'src/**/*.js'"
  }
}

Note the added single quotes above. They make sure that the shell does not try to expand the globs. Instead, the exact string src/**/*.js will be sent to prettier (as process.argv[2]). Then, prettier can pass that string to some glob library, such as glob, and let it expand the glob in a consistent way. I think this would be a nice improvement to the prettier CLI.

lydell added a commit to lydell/prettier that referenced this issue Jan 21, 2017
@vjeux
Copy link
Contributor

vjeux commented Jan 21, 2017

I think we should change the README to avoid using the glob expansion since it doesn't work in bash. We can replace it find find src -name '*.js' | xargs prettier but this doesn't feel super nice for people that don't know bash that well and doesn't work on windows.

I agree that passing globs as strings and having prettier handle them seem like a great solution.

@vjeux vjeux added the type:bug Issues identifying ugly output, or a defect in the program label Jan 21, 2017
jlongster pushed a commit that referenced this issue Jan 23, 2017
* Add glob support to the CLI

Fixes #145. Fixes #362.

* Update README.md regarding globs

* Use exact dependency for glob
@metacoding
Copy link

Thanks for your explanations but none of the above patterns works in Windows. Can you give me a hint what should I do to introduce all js files in src directory to prettier?

@hawkrives
Copy link
Contributor

@metacoding If you wrap your pattern in quotes, Prettier will execute the pattern for you in a cross-platform way.

prettier "src/**/*.js" will rub prettier on every file within a "src/" folder, for instance.

I believe there is some information in the docs about this, but I'm not sure where it is at the moment. (apologies).

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 5, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 5, 2018
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. type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

No branches or pull requests

5 participants