consolidation / output-formatters

Apply transformations to structured data to write output in different formats.
Other
192 stars 13 forks source link

CsvFormatter::csvEscape should provide optional enclosure and escape character arguments. #78

Closed aaronsenecal closed 5 years ago

aaronsenecal commented 5 years ago

Currently CsvFormatter::csvEscape only supports fputcsv's default $enclosure and $escape_char arguments. We've encountered a few cases where it'd be helpful to be able to configure these arguments (especially $escape_char) as formatter options.

For example: some of our CSV data contain double quotes that are immediately preceded by a backslash. Using its default $escape_char value, fputcsv fails to generate a spec-compliant CSV (leaving slash-prefixed quotes in enclosed fields as \", when it should be correcting them to \"". Specifying the NUL character for this argument allows us to work around this issue.

Of course, a fix for this issue should respect PHP's defaults for fputcsv, and any additional arguments exposed by this fix should be strictly optional so as to maintain backwards compatibility.

greg-1-anderson commented 5 years ago

If fputcsv does not produce correct output, perhaps it should be replaced with code that is correct.

Regarding making the escape character and enclosure parameters configurable is also reasonable. I am unlikely to implement this myself, but a PR would be welcome.

aaronsenecal commented 5 years ago

@greg-1-anderson, I'm with you 100% on the fputcsv issue. It sounds like PHP 7.4 will provide us with the ability to pass an empty string for fputcsv's $escape_char argument, which will functionally disable the proprietary filtering mechanism that causes the quirky behavior mentioned in my example above.

In the meantime, though PR #79 is currently passing all tests (with the exception of the appvoyeur test that's currently failing on master) and is ready for review whenever you get the chance.

Thanks for taking a look!