fix(csv): strip the UTF-8 BOM and skip blank rows before building the table - #2303
Open
Ruiming Zhao (uuzzrm) wants to merge 1 commit into
Open
fix(csv): strip the UTF-8 BOM and skip blank rows before building the table#2303Ruiming Zhao (uuzzrm) wants to merge 1 commit into
Ruiming Zhao (uuzzrm) wants to merge 1 commit into
Conversation
… table Two things broke real-world CSV conversion: Excel prepends a UTF-8 BOM that landed inside the first header cell, and a single leading or trailing blank line parsed as an empty row and shifted every column (a leading blank line destroyed the table entirely, since the empty row became the header). Strip the BOM and drop empty rows before deciding what the table looks like.
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.
Two things broke real-world CSV conversion:
| \ufeffname | age |).csv.readerparses a blank line as an empty row. A trailing blank line added a garbage| |row, and a leading blank line was worse: the empty row became the header, so| |+ a zero-column separator was emitted and every real column was truncated away - the whole table came out as| |lines.This change strips the BOM and drops empty rows before deciding what the table looks like. The existing behavior for genuinely empty input is unchanged (empty markdown).
Tests: six new cases in
tests/test_csv_converter.pycovering the BOM with and without a charset hint, leading/trailing/mid-file blank lines, and all-blank input. They fail against the previous code (the blank-line ones produced the broken tables above) and pass with the fix.ruff check,ruff format --check, andmypyare clean on the changed files.Note: cell escaping (pipes / embedded newlines inside quoted fields) is a separate issue, already covered by #1816 - this PR intentionally stays out of that.