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

When using types, why is it necessary to specify a noop files? #706

Closed
magicmark opened this issue Feb 21, 2018 · 2 comments
Closed

When using types, why is it necessary to specify a noop files? #706

magicmark opened this issue Feb 21, 2018 · 2 comments
Labels

Comments

@magicmark
Copy link
Contributor

magicmark commented Feb 21, 2018

@chriskuehl

I'd expect something like this to match my stuff:

-   repo: https://github.com/prettier/prettier
    sha: 1.10.2
    hooks:
    -   id: prettier
        name: prettier (scss, markdown)
        types: [markdown, scss]

As evidenced, it doesn't: https://i.fluffy.cc/mMkBxvs5XF8Lbf8bxxXFRxWClBk8Cg58.html

This is because we apparently also need to add a files: ''

Is this behavior intentional?

From reading the docs, it's not obvious that this is the case - maybe we could clarify this? :)

Thanks!

EDIT: Looks like this might just be #707

@asottile
Copy link
Member

Looks like there's ~3 misunderstandings on your part here -- I'll highlight them in the docs for you :)

Neither that config, nor that config with files: '' should match anything:

From this:

types and files are evaluated with AND when filtering. Tags within types are also evaluated using AND.

That is, if you have types: [scss, markdown] you're asking for all files which are both markdown and scss.

Additionally, defaults:

From this

(this is for .pre-commit-config.yaml)

files (optional) override the default pattern for files to run on.
types (optional) override the default file types to run on

That is, it'll inherit any value specified in the repository manifest and only if you override it will that change your outcome.

The defaults are as you'd expect. From this:

files: (optional: default '') the pattern of files to run on. new in 0.15.0: now optional.
types: (optional: default [file]) list of file types to run on.

That is, without specifying anything in .pre-commit-hooks.yaml or .pre-commit-config.yaml you'd match all files.

The third piece to the puzzle is the defaults provided by the manifest:

from here:

    # From https://github.com/prettier/prettier/blob/133303f47a30f6b3e46ffdf9d5c2d6609d65c416/src/options.js#L32-L42
    files: \.(css|less|scss|html|ts|tsx|graphql|gql|json|js|jsx)$

It's likely that this regex hasn't been updated as prettier has learned how to work on other file types. Probably worth a PR at the very least?

prettier originally used types, but it supports a wide variety of things and so it was switched to using files -- here's the relevant issue : prettier/prettier#2745

Putting it together, your manifest as written is asking for:

  • filenames that match \.(css|less|scss|html|ts|tsx|graphql|gql|json|js|jsx)$
  • AND files that are markdown
  • AND files that are scss

So an empty set.

You may also be interested in this issue, though there are no current plans or ideas for implementation: #607

Thanks for the issue -- hope this was helpful.

@magicmark
Copy link
Contributor Author

I see, thanks!

As suggested: prettier/prettier#4021

droctothorpe pushed a commit to droctothorpe/pre-commit that referenced this issue Mar 23, 2022
…e-config

[pre-commit.ci] pre-commit autoupdate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants