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 double quote escaping before new line #268

Merged
merged 1 commit into from
Mar 5, 2018
Merged

Fix double quote escaping before new line #268

merged 1 commit into from
Mar 5, 2018

Conversation

juanjoDiaz
Copy link
Collaborator

This is the proper fix for the issues reported on #267 with a couple test to ensure that there is no regressions.

@coveralls
Copy link

coveralls commented Mar 5, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling cb8f452 on juanjoDiaz:fix_double_quotes_before_new_line into e70aff1 on zemirco:master.

@@ -181,7 +181,7 @@ class JSON2CSVBase {
//a backslash, and it's not at the end of the stringifiedValue.
stringifiedValue = stringifiedValue
.replace(/^"(.*)"$/, this.opts.quote + '$1' + this.opts.quote)
.replace(/(\\")(?=.)/g, this.opts.doubleQuote)
.replace(/(\\")(?!$)/g, this.opts.doubleQuote)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming your tests pass, this fix is good with me. Although I thought $ would match both end of line and end of string. Or does it only match end of string?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like $ only matches line break if the multiline flag (m) is true (which it's not). So this looks good to me.

@knownasilya knownasilya merged commit fa991cf into zemirco:master Mar 5, 2018
@knownasilya
Copy link
Collaborator

Published as v4.0.1

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.

4 participants