Skip to content
This repository has been archived by the owner on Jun 28, 2021. It is now read-only.

Automatic rowDelimiter discovery should ignore newlines in quoted fields #142

Closed
ctstone opened this issue Jul 20, 2017 · 8 comments
Closed

Comments

@ctstone
Copy link

ctstone commented Jul 20, 2017

Repro:

const content = 'one\ttwo\t"three.1\nthree.2"\r\nfour\tfive\tsix\r\n';
const options = {delimiter: '\t'}
csvParse(content, options, (err, data) => {
    if (err) throw err;
    console.log(data);
});

Results in Error: Invalid closing quote at line 2; found "\r" instead of delimiter "\t". I get the the same error if I explicitly set rowDelimiter to \n in my options.

I would expect the auto-discovery to ignore the first newline it finds (\n) since it occurs in a quoted value.

The problem goes away when the correct value is explicitly set in options, {delimiter:'\t', rowDelimiter:'\r\n'}.

@ctstone
Copy link
Author

ctstone commented Jul 20, 2017

Also, am I mistaken or are the constants 'auto', 'unix', 'mac', 'windows', 'unicode' never recognized for rowDelimiter, and instead treated as literals?

@wdavidw
Copy link
Member

wdavidw commented Jul 23, 2017

They are literal and are converted to rowDelimiter, for exemple, the value "windows" is converted to "\r\n".

@wdavidw
Copy link
Member

wdavidw commented Jul 23, 2017

When I execute your sample, I get

[ [ 'one', 'two', 'three.1\nthree.2' ],
  [ 'four', 'five', 'six' ] ]

Which seems to be the expected behavior unless i'm missing something.

@ctstone
Copy link
Author

ctstone commented Jul 24, 2017

Are you sure you are running the same code? Here is an online repro:
https://runkit.com/ctstone/csv-parse-row-delimiter-error

RE: constants, the string literal 'windows' does not appear in source:
https://github.com/wdavidw/node-csv-parse/search?utf8=%E2%9C%93&q=windows&type=

so i'm wondering where the conversion happens.

In fact, if I peek at the parser's options I can see that the value is still 'windows':

> let options = {delimiter: '\t', rowDelimiter: 'windows'}
> console.log(csvParse(content, options, () => null).options.rowDelimiter)
[ 'windows' ]

And another repro showing that conversion is not happening:
https://runkit.com/ctstone/csv-parse-rowdelimiter-constants

@wdavidw
Copy link
Member

wdavidw commented Jul 24, 2017

This test pass for me: https://runkit.com/ctstone/csv-parse-row-delimiter-error
I'm using version 1.2.1 of csv-parse.

@wdavidw
Copy link
Member

wdavidw commented Jul 24, 2017

About the literal, I was wrong, I'm myself got confused. I believed it was part of a very old version of the parser and the doc was never updated to reflect its removal.

@ctstone
Copy link
Author

ctstone commented Jul 24, 2017

1.2.1 works for me as well, but is not yet published to NPM

Looks like a lot changed from 1.2.0. Guessing it's this line that fixed it.

@wdavidw
Copy link
Member

wdavidw commented Jul 24, 2017

Sorry, it should have been published. It is now.

@ctstone ctstone closed this as completed Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants