fix(compress): pin utf-8 and newline on file I/O - #763
Open
francojeferson wants to merge 1 commit into
Open
Conversation
Path.read_text / Path.write_text default to the locale encoding and to CRLF translation on Windows. Two user-visible consequences: - Non-ASCII output is written in the locale codec. An em dash lands as a bare 0x97 under cp1252, so the file stops being valid UTF-8. validate.py read it back through the same default and reported success, because the round-trip is self-consistent and wrong. - Every processed file gets its line endings rewritten to CRLF. A 67-line LF file came back 67 bytes larger. Pin encoding="utf-8" and newline="" on every read and write, then restore the source file's line endings on the way out since Claude always answers in LF. Reads go through open() rather than read_text(newline=...) because that parameter is 3.13+ and the skill supports 3.10+. Also normalize both sides of the no-op comparison. The body now carries the source's CRLF while Claude answers LF, so a raw compare would never match and the "output identical to input" guard would stop firing on CRLF files entirely. detect.py gets the same encoding fix. It only classifies, so it cannot corrupt a file, but on a non-UTF-8 locale it can misread prose as code and silently skip it. Refs JuliusBrussee#762, JuliusBrussee#686
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #762. Also covers #686 (and #652, #655, #533).
What
Path.read_text/Path.write_textdefault to the locale encoding and to CRLF translation on Windows. Both defaults mutate the user's file.Encoding. Non-ASCII output goes out in the locale codec. An em dash becomes a bare
0x97under cp1252, so the file stops being valid UTF-8.validate.pythen read it back through the same default, saw a correct em dash, and printedValidation passed— the round-trip is self-consistent and wrong, so the corruption ships silently.Line endings. Every processed file comes back CRLF. Measured on a real run: a 67-line LF file restored from backup at +67 bytes, one CR per line.
git diffthen reports every line changed.Overlap with #683
#683 already fixes the encoding half, and this PR contains that same change — the two touch identical call sites, so splitting them would just conflict. What this adds on top:
detect.py:93, which caveman-compress: missing encoding="utf-8" on file I/O corrupts non-ASCII files on Windows (data loss, no crash) #686's body lists as needing the fix but fix: use explicit utf-8 encoding in compress/validate file I/O #683 missesHappy to rebase down to newline-only if #683 lands first. No claim on that work intended.
Notes
open()rather thanread_text(newline=...). That parameter is 3.13+ andCLAUDE.mdstates the skill supports Python 3.10+.validate.pydeliberately does not getnewline="". The validator needs universal-newline normalization so a CRLF original and an LF candidate still compare equal — pinning it there would makevalidate_code_blocksfail on every CRLF file.compressed_body.strip() == body.strip()would never match and the "output identical to input" guard would stop firing on CRLF files entirely. The existingtest_identical_compressed_output_does_not_touch_diskcaught this — it is green again.Tests
New
tests/test_compress_encoding.py, 13 cases:match_line_endingsunit tests,read_exactround-trips, and end-to-end throughcompress_filewithcall_claudestubbed, so no API key is needed.tests.test_hooksreports 3 errors andnpm test2 failures on my Windows box. Both are identical on a clean checkout ofmainand unrelated to this change.CI will resync
plugins/caveman/skills/caveman-compress/scripts/on merge.