feat(csv): support fieldsPerRecord in CsvParseStream
#5600
Conversation
| if ("skipFirstRow" in testCase) { | ||
| options.skipFirstRow = testCase.skipFirstRow; | ||
| } | ||
| if ("columns" in testCase) { | ||
| options.columns = testCase.columns; | ||
| } | ||
| if ("trimLeadingSpace" in testCase) { | ||
| options.trimLeadingSpace = testCase.trimLeadingSpace; | ||
| } | ||
| if ("lazyQuotes" in testCase) { | ||
| options.lazyQuotes = testCase.lazyQuotes; | ||
| } | ||
| if ("fieldsPerRecord" in testCase) { | ||
| options.fieldsPerRecord = testCase.fieldsPerRecord; | ||
| } | ||
| if ("skipFirstRow" in testCase) { | ||
| options.skipFirstRow = testCase.skipFirstRow; | ||
| } | ||
| if ("columns" in testCase) { | ||
| options.columns = testCase.columns; | ||
| } |
There was a problem hiding this comment.
This adds fieldsPerRecord, and also reorders skipFirstRow and columns to match the order of properties declared in CsvParseStreamOptions so that it will be easy for us to notice the missing field in the future
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5600 +/- ##
=======================================
Coverage 96.37% 96.38%
=======================================
Files 466 466
Lines 37526 37559 +33
Branches 5528 5538 +10
=======================================
+ Hits 36167 36201 +34
+ Misses 1317 1316 -1
Partials 42 42 ☔ View full report in Codecov by Sentry. |
| error: { | ||
| klass: Error, | ||
| msg: | ||
| "Error number of fields line: 1\nNumber of fields found: 3\nExpected number of fields: 2", | ||
| }, | ||
| }, | ||
| { | ||
| name: "bad quote in bare field", | ||
| input: `a "word",1,2,3`, | ||
| errorMessage: "Error line: 1\nBad quoting", | ||
| error: { | ||
| klass: SyntaxError, | ||
| msg: | ||
| 'record on line 1; parse error on line 0, column 2: bare " in non-quoted-field', | ||
| }, | ||
| }, | ||
| { | ||
| name: "bad quote in quoted field", | ||
| input: `"wo"rd",1,2,3`, | ||
| errorMessage: "Error line: 1\nBad quoting", | ||
| error: { | ||
| klass: SyntaxError, | ||
| msg: | ||
| 'record on line 1; parse error on line 0, column 3: extraneous or missing " in quoted-field', | ||
| }, |
There was a problem hiding this comment.
In the previous test the expected error messages were not asserted. I fixed this and now the error class and message are checked - this reveals that these actual error messages feel a little bit unnatural, in the sense that the first sentence says "line 1" but the second says "parse error on line 0". I think this is inconsistent indexing.
This is not really related to what I'd like to cover in this PR, but maybe we'd like to address it before v1 too?
Although the constructor of
CsvParseStreamacceptsfieldsPerRecordoption (see https://jsr.io/@std/[email protected]/doc/~/CsvParseStreamOptions) that ensures that every record has the specified (or inferred from the first row) number of fields, this option has no effect at all in the current implementation.To fix this issue, this patch implements the
fieldsPerRecordlogic inCsvParseStreamtogether with sufficient amount of test cases.