fix(csv-stringify): quote fields containing a carriage return

[sarathfrancis90]

A field holding a lone carriage return is written unquoted, because the quoting check only looks for the configured record_delimiter (\n by default). The parser treats both \r and \n as record delimiters, so the value gets split apart and the document no longer round-trips:

const { stringify } = require("csv-stringify/sync");
const { parse } = require("csv-parse/sync");

parse(stringify([["a\rb"]]));
// => [["a"], ["b"]]   (expected [["a\rb"]])

\n and \r\n are already quoted; only a bare \r slips through.

The fix: when the record delimiter is newline-based, quote any field that contains \r or \n. A custom, non-newline delimiter still leaves line breaks as data, so that behaviour is unchanged.

Found while fuzzing the stringify → parse round trip. Added a regression test; the escape-formulas test is updated since a \r-leading field is now correctly quoted. Full csv-stringify suite (202) and tsc pass.

[wdavidw]

The detection shall be generic. The current implementation checks if value contains record_delimiter, which ever the characters and without treating \n and \r as special citizens. The problem arises from two different default behaviors between stringify and parse to handle default record delimiter. stringify default to the \n character. parse default to the discovery of \n, \r and \r\n.

[sarathfrancis90]

You are right, and sorry for the delay here.

Reworked along those lines: the quoting test stays generic and I moved the fix to the defaults, which is where the asymmetry you described actually is. normalize_options now derives the set of delimiters to quote against — the \r\n, \n, \r that parse discovers when the option is left out, or the configured delimiter alone once it is set. The detection is back to a plain "does the value contain one of them", with no special casing of \r and \n in the stringifier.

Kept it off stringifier.options since the exposed keys are asserted in api.types.ts; it is derived once in normalization and carried in the internal state. A cast returning its own record_delimiter gets the set derived from that one.

fields with linebreaks and different record delimiter already covers the custom-delimiter side, so I only added the \r case. Suite and types pass, and a 150k-record stringify → parse round trip comes back clean.

[wdavidw]

Instead of returning record_delimiter_defaulted, we shall create a new option quote_record_delimiter. It default to true unless options.record_delimiter is provided. This way we don't need to change the return signature of normalize_options, a dedicated test file is created and the option can be documented explaining that this is to preserve round trip calls between stringify and parse with default options.

[sarathfrancis90]

Done, that is a cleaner shape than what I had.

quote_record_delimiter is normalized just before record_delimiter, so it can still see whether you passed one: it defaults to true when the delimiter is left out and false once it is set. normalize_options keeps its two-value return, and with the flag living on the options there was no longer anything to thread through the state, so lib/index.js and sync.js are back to untouched. The stringifier reads it alongside record_delimiter and tests \n and \r, which covers the three sequences.

Setting it explicitly works in both directions — false opts out with the default delimiter, true opts in with a custom one — and the configured delimiter is always quoted against either way. Documented it on both Options and OptionsNormalized, and the new key is in the api.types assertion.

Tests are in test/option.quote_record_delimiter.ts, including the custom-delimiter defaults and the two explicit cases. Suite and types pass, and a 40k-record stringify → parse round trip is clean.

One thing worth flagging: if a cast returns its own record_delimiter, the merged options still carry the global quote_record_delimiter, so a cast delimiter combined with the default true quotes against \n and \r as well. That only ever quotes more than strictly needed, so it round-trips fine, but say the word if you would rather it re-derive per cast.

[sarathfrancis90]

Rebased on master — it had moved under me while I was writing that. The check now composes with your emits_separator rather than the plain indexOf I described above:

const containsRecordDelimiter =
  emits_separator(value, record_delimiter) ||
  (quote_record_delimiter === true &&
    (emits_separator(value, "\n") || emits_separator(value, "\r")));

Both are single characters so the fusion branch is inert for them, but it keeps the three checks reading the same way. 214 tests and the round trip still pass.

[wdavidw]

One thing worth flagging: if a cast returns its own record_delimiter, the merged options still carry the global quote_record_delimiter, so a cast delimiter combined with the default true quotes against \n and \r as well. That only ever quotes more than strictly needed, so it round-trips fine, but say the word if you would rather it re-derive per cast.

Could you illustrate this with a test and we'll be good.

[sarathfrancis90]

Added three, and writing them corrected what I said above.

The cast record_delimiter never reaches the record separator — that is read from this.options when the chunk is assembled, so a cast returning :: still emits \n between records. Quoting against \n and \r there is not extra, it is exactly what that output needs; what the cast adds is :: on top, which is the only part that is wider than strictly required.

So the cases are: a \r field still quoted through a cast (and the record separator visibly still \n), the cast delimiter itself quoted, and the same cast under a global record_delimiter leaving \r alone since the flag is false by then.

217 tests pass.