max-mapper / csv-write-stream

A CSV encoder stream that produces properly escaped CSVs
BSD 2-Clause "Simplified" License
204 stars 37 forks source link

boolean false serialized as empty string #8

Closed junosuarez closed 10 years ago

junosuarez commented 10 years ago

Currently in 0.1.1 the output of

csvWriter({headers: ['a','b','c','d']})
  .on('data', function (x) { process.stdout.write(x) })
  .write([true, false, 'true', 'false'])

is

a,b,c,d
true,,true,false

whereas I would expect

a,b,c,d
true,false,true,false

Can you confirm if this is working as designed? If not, would you accept a PR fixing this?

max-mapper commented 10 years ago

ahhh yes this is a bug, i'd happily accept a PR

junosuarez commented 10 years ago

What should the defined values of null and undefined serialize to? Currently those also error, and it seems to make sense to address all of them in the same patch. It seems sensible to me to serialize them as empty cells, eg:

[false,'false',0,null,undefined]

=>

false,false,0,,
max-mapper commented 10 years ago

agreed

null -> ''
undefined -> ''
false -> 'false'
junosuarez commented 10 years ago

Turns out the originally reported bug has been fixed in master (actually, after looking through the commit history, I can't find the offending line anywhere... strange... it's index.js:42 in the package), but it's still in the latest published version of the module (0.1.1). Always fun when my "failing test cases" pass mysteriously on the first run :) Putting together a patch for the null and undefined cases.