Fix invalid Python file encoding "utf8"#5252
Fix invalid Python file encoding "utf8"#5252nicoddemus merged 4 commits intopytest-dev:masterfrom tambry:utf8
Conversation
Since Python 3 it must be "utf-8", which is the official name. This is backwards compatible with Python 2.
|
Interesting - do you have a reference? There are more occurences of this however, e.g. in testing/test_warnings.py. |
Codecov Report
@@ Coverage Diff @@
## master #5252 +/- ##
==========================================
- Coverage 94.99% 91.94% -3.05%
==========================================
Files 115 115
Lines 26159 26199 +40
Branches 2578 2578
==========================================
- Hits 24849 24088 -761
- Misses 1003 1791 +788
- Partials 307 320 +13
Continue to review full report at Codecov.
|
|
I unfortunately do not have a reference. It seems to be a bit flaky whether the error happens. Though you can find cases of this problem on Google. I fixed only this occurence, because in my initial search only it showed up on Chromium CodeSearch. |
|
unless there's a stacktrace I'm inclined to leave this as-is -- PEP 263 seems to allow what we have currently and >>> codecs.lookup('utf8')
<codecs.CodecInfo object for encoding utf-8 at 0x7faf98f0fca0> |
|
Thanks @tambry! Normalized all encoding declarations to the same format, it doesn't hurt being consistent. 👍 |
| @@ -1,4 +1,4 @@ | |||
| # coding=utf8 | |||
There was a problem hiding this comment.
let's use fix-encoding-pragma from pre-commit/pre-commit-hooks
(yep, there's a pre-commit hook for everything!)
There was a problem hiding this comment.
Sure, sounds good! Could you please contribute that in place of this PR then? 😁
There was a problem hiding this comment.
Can it only do it if necessary maybe?
There was a problem hiding this comment.
for consistency it's all or none, that way you don't have to think about it
There was a problem hiding this comment.
Isn't the main point of pre-commit already that "you don't have to think about it", i.e. you do not have to care, and it will insert/request it if necessary?
There was a problem hiding this comment.
yeah so now you don't have to futz with whether a file has it or not, it always has it
|
hmmm, that's weird it's showing a diff in CI -- let me see what I can do there, sorry about that! |
I suspect crlf since it's windows and that's not implemented yet -- easy patch though: pre-commit/pre-commit-hooks#384 |
|
Thanks for continuing work on this! But I think this misses all the cases that are in the middle of code somewhere. Might be worth running grep and fixing those manually? |
yep, did that already -- did I miss some? |
Ah, probably fine then. 👍 |
|
Waiting for @blueyed's approval/change requests before merging. |
I don't like changing that many files unnecessarily (https://github.com/pytest-dev/pytest/pull/5252/files#r285079054). But feel free to ignore me, of course.. :) |
|
Personally I don't mind when the change is a single line too much; wide sweeping changes like formatting annoy me more as they make 'git blame' harder to use. 👍 😁 |
4 participants
Since Python 3 it must be "utf-8", which is the official name.
This is backwards compatible with Python 2.