Add some alternative line endings and whitespace checks

[peternewman]

Before we merge #1889

[vikivivi]

My suggestions.

• Adding both leading and trailing whitespace to the test.
• Adding newline='' parameter to open() will preserve the original End of Line (EOL) LF, CRLF, CR line terminators.

diff --git a/codespell_lib/tests/test_basic.py b/codespell_lib/tests/test_basic.py
index 6584613c..1972de93 100644
--- a/codespell_lib/tests/test_basic.py
+++ b/codespell_lib/tests/test_basic.py
@@ -100,15 +100,15 @@ def test_basic(tmpdir, capsys):
     os.remove(fname)
 
     with open(op.join(d, 'bad.txt'), 'w') as f:
-        f.write('abandonned\nAbandonned\nABANDONNED\nAbAnDoNnEd')
-    assert cs.main(d) == 4
+        f.write('abandonned\nAbandonned\nABANDONNED\nAbAnDoNnEd\nabandonned\rAbandonned\r\n ABANDONNED \nAbAnDoNnEd')  # noqa: E501
+    assert cs.main(d) == 8
     code, _, stderr = cs.main('-w', d, std=True)
     assert code == 0
     assert 'FIXED:' in stderr
-    with open(op.join(d, 'bad.txt')) as f:
+    with open(op.join(d, 'bad.txt'), newline='') as f:
         new_content = f.read()
     assert cs.main(d) == 0
-    assert new_content == 'abandoned\nAbandoned\nABANDONED\nabandoned'
+    assert new_content == 'abandoned\nAbandoned\nABANDONED\nabandoned\nabandoned\rAbandoned\r\n ABANDONED \nabandoned'  # noqa: E501
 
     with open(op.join(d, 'bad.txt'), 'w') as f:
         f.write('abandonned abandonned\n')

[peternewman]

* Adding both leading and trailing whitespace to the test.

Sounds sensible thanks @vikivivi .

* Adding `newline=''` parameter to `open()` will preserve the original End of Line (EOL) LF, CRLF, CR line terminators.

Ah I didn't know that, so it's just my tests that are broken hopefully.

Please could you add your comments as code suggestions to the PR, then you'll get a proper credit for them (and it's much easier for me to apply them 😆 ). See step 6 here https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request#adding-line-comments-to-a-pull-request

[peternewman]

Also it looks like we probably need to fix the codespell internals to open like this too, not just the tests!

[vikivivi]

• Adding both leading and trailing whitespace to the test.
• Adding newline='' parameter to open() will preserve the original End of Line (EOL) LF, CRLF, CR line terminators.
• The last word in the comparison is also corrected.

[vikivivi]

Also it looks like we probably need to fix the codespell internals to open like this too, not just the tests!

linux$ printf 'abandonned\nAbandonned\nABANDONNED\nAbAnDoNnEd\nabandonned\rAbandonned\r\n ABANDONNED \nAbAnDoNnEd' > test2.txt 
linux$ codespell -w test2.txt
linux$ cat test2.txt
linux$ cat test2.txt | xxd -p

abandoned
Abandoned
ABANDONED
abandoned
Abandoned
 ABANDONED 
abandoned

• Personally, it seem working normally to me by validating the hexdump. I don't see a fix to the internals is needed.
• 0x0A, 0x0D, 0x0D0A are bold

6162616e646f6e65640a4162616e646f6e65640a4142414e444f4e45440a
6162616e646f6e65640a6162616e646f6e65640d4162616e646f6e65640d
0a204142414e444f4e4544200a6162616e646f6e6564

[peternewman]

* Personally, it seem working normally to me by validating the hexdump. I don't see a fix to the internals is needed.

I was just a bit confused as there was no newline obvious in the internals. Indeed it's been removed:
dc39dfc

Also this is all still a bit more complicated and broken, as the AppVeyor based Windows tests now fail as they write everything out with a /r/n rather than honouring the newline option.

[vikivivi]

• I am surprised by Windows inconsistency with Linux Python. Maybe the root cause is at the write which write Windows \r\n, so it is consistent when read back with \r\n.
• What about opening in binary mode and compare using binary? The pytest still passed in Linux.
• Another area to look at is with open(op.join(d, 'bad.txt'), 'w') as f:, which might open and write text file using Windows CRLF line terminators. This leads to reading back with CRLF when set with newline='' when reading back. What about setting newline='' or mode='wb' when writing? Current mode is w when writing. If write in binary mode, you need to f.write() in binary as well.

diff --git a/codespell_lib/tests/test_basic.py b/codespell_lib/tests/test_basic.py
index 6584613c..cf5d1fae 100644
--- a/codespell_lib/tests/test_basic.py
+++ b/codespell_lib/tests/test_basic.py
@@ -99,16 +99,16 @@ def test_basic(tmpdir, capsys):
     assert 'cannot find dictionary' in stderr
     os.remove(fname)
 
-    with open(op.join(d, 'bad.txt'), 'w') as f:
-        f.write('abandonned\nAbandonned\nABANDONNED\nAbAnDoNnEd')
-    assert cs.main(d) == 4
+    with open(op.join(d, 'bad.txt'), 'wb') as f:
+        f.write(b'abandonned\nAbandonned\nABANDONNED\nAbAnDoNnEd\nabandonned\rAbandonned\r\nABANDONNED \n AbAnDoNnEd')  # noqa: E501
+    assert cs.main(d) == 8
     code, _, stderr = cs.main('-w', d, std=True)
     assert code == 0
     assert 'FIXED:' in stderr
-    with open(op.join(d, 'bad.txt')) as f:
+    with open(op.join(d, 'bad.txt'), mode='rb') as f:
         new_content = f.read()
     assert cs.main(d) == 0
-    assert new_content == 'abandoned\nAbandoned\nABANDONED\nabandoned'
+    assert new_content == b'abandoned\nAbandoned\nABANDONED\nabandoned\nabandoned\rAbandoned\r\nABANDONED \n abandoned'  # noqa: E501
 
     with open(op.join(d, 'bad.txt'), 'w') as f:
         f.write('abandonned abandonned\n')

[peternewman]

• I am surprised by Windows inconsistency with Linux Python. Maybe the root cause is at the write which write Windows \r\n, so it is consistent when read back with \r\n.
  • Another area to look at is with open(op.join(d, 'bad.txt'), 'w') as f:, which might open and write text file using Windows CRLF line terminators. This leads to reading back with CRLF when set with newline='' when reading back. What about setting newline='' or mode='wb' when writing?

Yeah these two did the trick, changing to actually writing out the correct newlines means the comparison then works as expected!

[peternewman]

Likewise I'd agree having generated a file using:
echo -e "abandonned\nAbandonned\nABANDONNED\nAbAnDoNnEd\nabandonned\rAbandonned\r\nABANDONNED \n AbAnDoNnEd" > lineending.txt

And inspected with:
hexdump -C lineending.txt | grep " 2*0[ad]*"

Indeed it doesn't seem to change endings/whitespace for me when run via Codespell as:
python3 -m codespell_lib -w lineending.txt

[vikivivi]

LGTM. pytest working OK in my Ubuntu Conda environment.

[peternewman]

Feel free to merge when you're happy @larsoner .

[larsoner]

Thanks @peternewman