search

doowb / converter

Library to convert to or from JSON, YAML, XML, PLIST or CSV.
MIT License
42 stars 9 forks source link

JsonToCSV options ignored #21

Open VincenzoAsciolla opened 6 years ago

VincenzoAsciolla commented 6 years ago

Hi, i'm using this lib in order to convert all my i18n json translations files into one CSV. So, i need to use the JsonToCsv converter, using the semicolon as column separator. I noticed that if i pass a custom options object to the lib in order to customize the column separator, this is ignored... Going deep into the code, i noticed that the last commit https://github.com/doowb/converter/commit/4d0f376edb3b1970484d9d98112557b0805f4386 has introduced a bug for the JsonToCsv transformation, ignoring the options provided as input.

That commit changed the following code:

https://github.com/doowb/converter/blob/4d0f376edb3b1970484d9d98112557b0805f4386/src/convert.js#L57-L62

with the following, that ignore the input options parameter:

https://github.com/doowb/converter/blob/e4ba9e0a2c20a15fab97c85938b7d9e3fa62807b/src/convert.js#L56-L64

I verified that the bug could be solved simply passing the options.csv param to the .from() call, as already done in the following code:

https://github.com/doowb/converter/blob/e4ba9e0a2c20a15fab97c85938b7d9e3fa62807b/src/convert.js#L47-L54

Could this fix be applied to the project?

doowb commented 6 years ago

Thanks for the issue. Would you like to submit a PR with the change?

VincenzoAsciolla commented 6 years ago

Sorry, that was a my mistake. Actually, the problem is that i'm using the library gulp-convert, that depends explicitly on the version 0.5.0 of the converter lib. The bug i was talking about yesterday exists in that version! But i saw that the master branch has gone forward respect to the tag 0.5.0, fixing the bug...

In this case, i can solve my problem forcing the dependency to the master branch of your project in my package.json...

Anyway, it would be better to creare a new tag to the converter project, updating consequently the dependency version in the gulp-convert project. But i understand these are different projects... So it could be hard. What do you suggest?