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 types_or #1677

Merged
merged 1 commit into from Nov 3, 2020
Merged

Add types_or #1677

merged 1 commit into from Nov 3, 2020

Conversation

@MarcoGorelli
Copy link
Contributor

@MarcoGorelli MarcoGorelli commented Nov 2, 2020

closes #607

@MarcoGorelli MarcoGorelli marked this pull request as draft Nov 2, 2020
@MarcoGorelli MarcoGorelli force-pushed the MarcoGorelli:types_or branch 2 times, most recently from e5b7b12 to b328a3e Nov 2, 2020
@MarcoGorelli MarcoGorelli marked this pull request as ready for review Nov 2, 2020
@MarcoGorelli MarcoGorelli marked this pull request as draft Nov 2, 2020
@MarcoGorelli MarcoGorelli marked this pull request as ready for review Nov 2, 2020
Copy link
Member

@asottile asottile left a comment

good start!

I believe check_hooks_apply and check_useless_excludes need to be updated

frozenset(types),
frozenset(types_or),
frozenset(exclude_types),
)

This comment has been minimized.

@asottile

asottile Nov 2, 2020
Member

this should be three separate lines

ret = []
for filename in names:
tags = self._types_for_file(filename)
if tags >= types and not tags & exclude_types:
if tags >= types and not tags & exclude_types and tags & types_or:

This comment has been minimized.

@asottile

asottile Nov 2, 2020
Member

teeny tiny thing, I'd order the conditions the same as the parameters

hook['types'],
hook['types_or'],
hook['exclude_types'],
)

This comment has been minimized.

entry: bin/hook.sh
language: script
types: [file]
types_or: [python, cython]

This comment has been minimized.

@asottile

asottile Nov 2, 2020
Member

these are usually easier to demo with a local repo

I'd probably not write a full integration test for this feature as well but it looks like there maybe aren't great examples using the matcher

This comment has been minimized.

@MarcoGorelli

MarcoGorelli Nov 2, 2020
Author Contributor

Thanks for your feedback, but I'm not sure I understand - are there any existing tests I should look at as examples? If not, I'll try to figure this out, no worries

@@ -0,0 +1,3 @@
#!/usr/bin/env bash
echo $@

This comment has been minimized.

@asottile

asottile Nov 2, 2020
Member

~technically this should be echo "$@"

@MarcoGorelli MarcoGorelli force-pushed the MarcoGorelli:types_or branch from b328a3e to 62f668f Nov 2, 2020
@MarcoGorelli
Copy link
Contributor Author

@MarcoGorelli MarcoGorelli commented Nov 2, 2020

good start!

Thanks!

I believe check_hooks_apply and check_useless_excludes need to be updated

Sorry, this isn't clear to me - I updated check_useless_excludes, and it's not obvious to me how check_hooks_apply needs to be updated, any hints?

@asottile
Copy link
Member

@asottile asottile commented Nov 2, 2020

I believe check_hooks_apply and check_useless_excludes need to be updated

Sorry, this isn't clear to me - I updated check_useless_excludes, and it's not obvious to me how check_hooks_apply needs to be updated, any hints?

oh maybe I'm wrong, maybe it already works

Copy link
Member

@asottile asottile left a comment

@asottile asottile merged commit 3fa9d3d into pre-commit:master Nov 3, 2020
2 checks passed
2 checks passed
pre-commit.ci - pr checks completed successfully
Details
pre-commit.pre-commit #20201102.5 succeeded
Details
@MarcoGorelli MarcoGorelli deleted the MarcoGorelli:types_or branch Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants