Fix #326: Handle CRLF in end-of-file-fixer

[mtkennerly]

I'm not sure if it's a problem that this approach always replaces a final \r\n with \n, if that could conflict with the mixed-line-ending hook or Git settings. On one hand, the existing implementation always wrote \n if there was no final line break, so this is consistent with that, but then another hook could convert that \n to \r\n, and this hook would have just ignored it.

[asottile]

hmmm, it would be better if it kept the \r\n at the end -- is it difficult to adjust the logic to do that?

[asottile]

can probably do something like:

if eof.count(b'\r') == len(eof):
    # append b'\r' and truncate
elif eof.count(b'\n') < len(eof):
    # append b'\r\n' and truncate
else:
    # append b'\n' and truncate

[mtkennerly]

Thanks for the quick response on this. I'll get it updated.

[asottile]

lmk and I can just push it, here's a patch on top of yours which supports \r linebreaks as well:

diff --git a/pre_commit_hooks/end_of_file_fixer.py b/pre_commit_hooks/end_of_file_fixer.py
index 37ebbb5..5ab1b7b 100644
--- a/pre_commit_hooks/end_of_file_fixer.py
+++ b/pre_commit_hooks/end_of_file_fixer.py
@@ -15,13 +15,13 @@ def fix_file(file_obj):
         return 0
     last_character = file_obj.read(1)
     # last_character will be '' for an empty file
-    if last_character != b'\n' and last_character != b'':
+    if last_character not in {b'\n', b'\r'} and last_character != b'':
         # Needs this seek for windows, otherwise IOError
         file_obj.seek(0, os.SEEK_END)
         file_obj.write(b'\n')
         return 1
 
-    while last_character == b'\n' or last_character == b'\r':
+    while last_character in {b'\n', b'\r'}:
         # Deal with the beginning of the file
         if file_obj.tell() == 1:
             # If we've reached the beginning of the file and it is all
@@ -38,8 +38,10 @@ def fix_file(file_obj):
     # newlines.  If we find extraneous newlines, then backtrack and trim them.
     position = file_obj.tell()
     remaining = file_obj.read()
-    for sequence in [b'\n', b'\r\n']:
-        if remaining.startswith(sequence) and len(remaining) > len(sequence):
+    for sequence in (b'\n', b'\r\n', b'\r'):
+        if remaining == sequence:
+            return 0
+        elif remaining.startswith(sequence):
             file_obj.seek(position + len(sequence))
             file_obj.truncate()
             return 1
diff --git a/tests/end_of_file_fixer_test.py b/tests/end_of_file_fixer_test.py
index deedeab..f8710af 100644
--- a/tests/end_of_file_fixer_test.py
+++ b/tests/end_of_file_fixer_test.py
@@ -17,6 +17,8 @@ TESTS = (
     (b'\xe2\x98\x83', 1, b'\xe2\x98\x83\n'),
     (b'foo\r\n', 0, b'foo\r\n'),
     (b'foo\r\n\r\n\r\n', 1, b'foo\r\n'),
+    (b'foo\r', 0, b'foo\r'),
+    (b'foo\r\r\r\r', 1, b'foo\r'),
 )
 

[asottile]

Thanks again for the report and the fix 🎉