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

Attempt to add fixing of BOMs #522

Merged
merged 1 commit into from
Oct 11, 2020

Conversation

jgowdy
Copy link
Contributor

@jgowdy jgowdy commented Oct 5, 2020

I'm attempting to add automatic fixing of BOMs

@jgowdy jgowdy marked this pull request as draft October 5, 2020 16:39
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

I think we should deprecate check-byte-order-marker, but we cannot remove it in this PR

pre_commit_hooks/byte_order_marker.py Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
@jgowdy jgowdy marked this pull request as ready for review October 5, 2020 16:56
@jgowdy jgowdy requested a review from asottile October 5, 2020 16:56
@jgowdy
Copy link
Contributor Author

jgowdy commented Oct 5, 2020

Sorry for the churn, I don't write much Python

@jgowdy jgowdy marked this pull request as draft October 5, 2020 17:09
@jgowdy jgowdy marked this pull request as ready for review October 5, 2020 17:10
@jgowdy jgowdy force-pushed the byte-order-marker-fix branch 5 times, most recently from c8ef07f to f3cee4e Compare October 5, 2020 19:54
@jgowdy
Copy link
Contributor Author

jgowdy commented Oct 5, 2020

Okay, I think this is good now.

@jgowdy
Copy link
Contributor Author

jgowdy commented Oct 8, 2020

Would you mind adding a hacktoberfest-accepted label to this PR? I didn't make this change specifically for hacktoberfest, I need it, but I am participating in hacktoberfest. Thanks!

@asottile
Copy link
Member

asottile commented Oct 8, 2020

yeah sorry I've been slow about this -- I'll take a look when I get the chance 👍

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@@ -17,8 +17,8 @@
language: python
types: [python]
- id: check-byte-order-marker
name: Check for byte-order marker
description: Forbid files which have a UTF-8 byte-order marker
name: 'check BOM - deprecated: use fix-byte-order-marker'
Copy link
Member

Choose a reason for hiding this comment

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

I shortened the name here so it doesn't wrap -- I should probably add a test for this

@@ -131,6 +131,12 @@
entry: file-contents-sorter
language: python
files: '^$'
- id: fix-byte-order-marker
Copy link
Member

Choose a reason for hiding this comment

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

I renamed this to fix-byte-order-marker so it's more clear what it does


def main(argv: Optional[Sequence[str]] = None) -> int:
parser = argparse.ArgumentParser()
parser.add_argument('filenames', nargs='*', help='Filenames to check')
Copy link
Member

Choose a reason for hiding this comment

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

I removed the --fix=no -- I'd rather have formatters-only


for filename in args.filenames:
with open(filename, 'rb') as f_b:
bts = f_b.read(3)
Copy link
Member

Choose a reason for hiding this comment

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

there was a leaking file descriptor here, I fixed that

@asottile asottile merged commit 08d1901 into pre-commit:master Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants