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

Excel issue needs option to force delimeter.wrap #188

Closed
1 task
peacechen opened this issue May 20, 2021 · 4 comments · Fixed by #189
Closed
1 task

Excel issue needs option to force delimeter.wrap #188

peacechen opened this issue May 20, 2021 · 4 comments · Fixed by #189

Comments

@peacechen
Copy link
Contributor

Excel automatically converts false boolean types to FALSE. Users who load and edit the CSV in Excel end up with booleans values that are uppercase TRUE/FALSE instead of lowercase. This does not parse back properly with csv2json. If I manually edit the CSV and wrap the boolean true/false in quotes, it remains lowercase after editing in Excel.

The delimeter.wrap option has no effect for all the types that I've tried. This feature had been requested in #133 but wasn't implemented.

The alternative would be to modify csv2json to be able to parse back TRUE/FALSE boolean values. That would be messier though.

Background Information

  • Module Version: 3.10.3
  • Node/Browser Version: Chrome 86

The issue I'm reporting is with:

  • [*] json2csv
  • csv2json

I have...

  • [*] searched to see if an issue has already been reported.
  • [*] verified that my JSON/CSV data is valid (using something like http://jsonlint.com or https://csvlint.io/).
  • [*] tried upgrading to the latest version of json-2-csv (since the issue may already be fixed).

Expected Behavior

"true", "false"

Actual Behavior

true, false

Data Sample

CSV:

MY_BOOL,FIELD1,FIELD2
true,data1,data2
true,data3,data4

Code Example

// Please include a simple example to replicate the issue
let converter = require('json-2-csv');

json2csv(data, onCompleted, {
  delimiter: {
    wrap: "\"",
  },
});
mrodrig added a commit that referenced this issue May 20, 2021
In order to support the use case of keeping Booleans as String values
that can easily be converted back with csv2json, this commit adds a new
option `wrapBooleans` which will allow users a way to make this possible

Fixes #188
mrodrig added a commit that referenced this issue May 20, 2021
In order to support the use case of keeping Booleans as String values
that can easily be converted back with csv2json, this commit adds a new
option `wrapBooleans` which will allow users a way to make this possible

Fixes #188
mrodrig added a commit that referenced this issue May 20, 2021
In order to support the use case of keeping Booleans as String values
that can easily be converted back with csv2json, this commit adds a new
option `wrapBooleans` which will allow users a way to make this possible

Fixes #188
mrodrig added a commit that referenced this issue May 20, 2021
In order to support the use case of keeping Booleans as String values
that can easily be converted back with csv2json, this commit adds a new
option `wrapBooleans` which will allow users a way to make this possible

Fixes #188
@mrodrig
Copy link
Owner

mrodrig commented May 20, 2021

Thanks for reporting this @peacechen. I can definitely see the use case for forcing boolean values to be wrapped since Excel is interpreting them when not wrapped in quotes. I've added a wrapBooleans options that will force these Boolean values to be wrapped in the options.delimiter.wrap value and released it in version 3.12.0 just now. Hope this helps.

@peacechen
Copy link
Contributor Author

peacechen commented May 20, 2021

Thanks for the fast turn-around!

A small TS issue: passing in wrapBooleans to options results in an error

Argument of type '{ wrapBooleans: boolean; }' is not assignable to parameter of type 'IFullOptions'.
Object literal may only specify known properties, and 'wrapBooleans' does not exist in type 'IFullOptions'.ts(2345)

It looks like wrapBooleans needs to be added to the TS definition for IFullOptions.

export interface IFullOptions extends ISharedOptions {

Update: I created a PR #190 to add wrapBooleans to the TS definition. Since the delimiter options were in ISharedOptions, I placed it in there to keep them together.
I tested the modified TS definition locally in node_modules and confirmed it works.

@mrodrig
Copy link
Owner

mrodrig commented May 20, 2021

Sorry about that - I'll merge your PR and will get that released in 3.13.0 in a few minutes. Thanks for the PR!

@peacechen
Copy link
Contributor Author

Thanks again for the fast merge and release 😄

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 a pull request may close this issue.

2 participants