Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix backslash logic #222

Merged
merged 1 commit into from
Jan 21, 2018
Merged

Fix backslash logic #222

merged 1 commit into from
Jan 21, 2018

Conversation

juanjoDiaz
Copy link
Collaborator

The issues is that line.replace(/\\\\/g,'\\') is performed twice during the CSV. And the test are tweaked to pass with this weird logic.

In the the should parse JSON values with trailing backslashes test for example:
The JSON file contains "Audi\\\\". However, "Audi\" is expected in the resulting CSV.
I would expect the correct escaped value to be "Audi\\"

Or the should not escape quotes with double quotes, when there is a backslash in the end test receives as input

[
  {"a string": "with a description"},
  {"a string": "with a description and \"quotes and backslash\\"},
  {"a string": "with a description and \"quotes and backslash\\\\"}
]

However, it expects and output like:

"a string"
"with a description"
"with a description and ""quotes and backslash\"
"with a description and ""quotes and backslash\"

It seems completely wrong that 2 strings that are different in the JSON are yielding exactly the same values in the CSV.
I would expect the final row to be "with a description and ""quotes and backslash\\"

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f700c4c on juanjoDiaz:bugfix/fix_backslash_logic into 9c861ed on zemirco:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f700c4c on juanjoDiaz:bugfix/fix_backslash_logic into 9c861ed on zemirco:master.

@coveralls
Copy link

coveralls commented Jan 21, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling f700c4c on juanjoDiaz:bugfix/fix_backslash_logic into 9c861ed on zemirco:master.

@knownasilya
Copy link
Collaborator

Good catch! I never noticed that!

@knownasilya knownasilya merged commit 29e9445 into zemirco:master Jan 21, 2018
@juanjoDiaz juanjoDiaz deleted the bugfix/fix_backslash_logic branch January 21, 2018 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants