Skip to content

Commit f0f1e8c

Browse files
jinmayjinmay
committed
csv: address review feedback
- Remove the redundant per-branch sentinel terminator setup in to_writer; the unconditional terminator call after the match is the single source. - Reword the empty-lineterminator errors to "must not be empty" (the constraint is non-empty, not single-character) on both entry points. - Promote the sentinel invariant check in writerow from debug_assert_eq! to assert_eq! so it also guards release builds. - Add a snippet case for a field containing a line-break byte to ensure the csv-core path drops only the trailing terminator.
1 parent f297947 commit f0f1e8c

2 files changed

Lines changed: 14 additions & 7 deletions

File tree

‎crates/stdlib/src/csv.rs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ mod _csv {
231231
.to_str()
232232
.ok_or_else(|| new_csv_error(vm, r#""lineterminator" must be a string"#))?;
233233
if value.is_empty() {
234-
return Err(new_csv_error(vm, r#""lineterminator" must be a string"#));
234+
return Err(new_csv_error(vm, r#""lineterminator" must not be empty"#));
235235
}
236236
Ok(value.to_owned())
237237
}
@@ -626,7 +626,7 @@ mod _csv {
626626
// non-empty string, including multi-character ones, is stored.
627627
if value.is_empty() {
628628
return Err(vm
629-
.new_type_error(r#""lineterminator" must be a 1-character string"#)
629+
.new_type_error(r#""lineterminator" must not be empty"#)
630630
.into());
631631
}
632632
res.lineterminator = Some(value.to_owned());
@@ -867,8 +867,7 @@ mod _csv {
867867
if let Some(dialect) = g.get(name) {
868868
let mut builder = builder
869869
.delimiter(dialect.delimiter)
870-
.double_quote(dialect.doublequote)
871-
.terminator(Terminator::Any(CSV_CORE_TERMINATOR_SENTINEL));
870+
.double_quote(dialect.doublequote);
872871

873872
if let Some(t) = dialect.quotechar {
874873
builder = builder.quote(t);
@@ -884,8 +883,7 @@ mod _csv {
884883
DialectItem::Obj(obj) => {
885884
let mut builder = builder
886885
.delimiter(obj.delimiter)
887-
.double_quote(obj.doublequote)
888-
.terminator(Terminator::Any(CSV_CORE_TERMINATOR_SENTINEL));
886+
.double_quote(obj.doublequote);
889887

890888
if let Some(t) = obj.quotechar {
891889
builder = builder.quote(t);
@@ -1584,7 +1582,7 @@ mod _csv {
15841582
// closing the final quote / emitting an empty record as needed).
15851583
// Drop that sentinel byte and append the real, possibly
15861584
// multi-character, line terminator.
1587-
debug_assert_eq!(buffer[buffer_offset - 1], CSV_CORE_TERMINATOR_SENTINEL);
1585+
assert_eq!(buffer[buffer_offset - 1], CSV_CORE_TERMINATOR_SENTINEL);
15881586
let mut output = buffer[..buffer_offset - 1].to_vec();
15891587
output.extend_from_slice(self.dialect.lineterminator.as_bytes());
15901588

‎extra_tests/snippets/stdlib_csv.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,15 @@ def test_multichar_lineterminator():
219219
)
220220
assert nonnum.getvalue() == '"a",1!@#', nonnum.getvalue()
221221

222+
# A field that itself contains a line-break byte must be kept intact: the
223+
# csv-core path drops only the trailing record terminator, not a byte from
224+
# the field data.
225+
embedded = io.StringIO()
226+
csv.writer(embedded, lineterminator="!@#", quoting=csv.QUOTE_ALL).writerow(
227+
["x\ny", "z"]
228+
)
229+
assert embedded.getvalue() == '"x\ny","z"!@#', embedded.getvalue()
230+
222231
# QUOTE_NONE escapes any byte of the terminator.
223232
none = io.StringIO()
224233
csv.writer(

0 commit comments

Comments
 (0)