Write generated data files as UTF-8 with LF instead of the locale encoding - #1238
Open
ppcvote wants to merge 1 commit into
Open
Write generated data files as UTF-8 with LF instead of the locale encoding#1238ppcvote wants to merge 1 commit into
ppcvote wants to merge 1 commit into
Conversation
…oding doctools opened every generated artifact in text mode with no encoding, so the descriptions' curly quotes were written in the host code page. Those code points exist in cp950 and cp1252, so the write succeeded and emitted non-UTF-8 bytes with no warning. The CSV writer also lacked newline="", turning to_csv's line endings into \r\r\n. Running the pre-commit hook on a cp950 host modified 185 files under data/, 34 of which stopped being valid UTF-8. With this change the same run leaves data/ byte-identical.
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.
Every generated-artifact writer opened its file in text mode with no
encoding=, so it used the host's locale encoding. The descriptions' curly quotes exist in cp950 and cp1252, so those writes succeeded and emitted non-UTF-8 bytes with no warning and exit 0. The CSV writer separately lackednewline="", soto_csv's line endings became\r\r\n.That matters because the
doctools-regeneratehook andrun_doctools.yml's own failure message both tell a contributor to regenerate and pushdata/.Running the hook on a cp950 host, before and after:
data/modified\r\r\nSeven one-argument changes across
doctools.py,md_gen.pyanddecision_tables/helpers.py:encoding="utf-8"everywhere,newline="\n"on the text writers so output stays LF, andnewline=""plus an explicitlineterminatoron the two CSV writers. Nothing underdata/is committed here, sorun_doctools.ymlregenerates and verifies on its own runner.The test runs a full generation under
-X warn_default_encoding(PEP 597) and fails on any EncodingWarning raised from inside the package. That is what makes it work on your UTF-8 CI: asserting on the generated bytes would pass there whether or not the defect is present. It fails on the current code namingdoctools.py:345and:197, and passes with the change.black --checkis clean. The four failures intest_dp_helpers.pyandtest_mixins.pyare present on an unmodified checkout here too.