CR line separators not handled
juuxstar opened this issue · 4 comments
When the CSV has lines that are separated by a single CR, it does not seem to handle them correctly. Instead of detecting it as the end of line, it keeps parsing the start of the next token on the next line as part of the last token on the previous line.
Definitely a bug here. While I did not intend to support CR by itself (the spec states CRLF,), this is something that would be nice to have in non-strict mode. I confess that this is hard to test on my mac, where the problem was that all native text files are just LF, something that was added as an optional behavior.
I'll look into adding this as part of the default mode (but not strict). On a related note, the unit test I just worked up caught that "1,2,3\r\n4,5,6" is evaluating to [ [1,2,"3\r"], [4,5,6] ] which is definitely wrong, so I'll fix that while I'm at it.
Nice thanks. Yeah, I already ran into the extra CR at the end of the token but use a .trim() to deal with it in the application layer.
I also noticed that the project does not include any unit tests. This would definitively help with testing and ensuring nothing got broken as time goes on. I personally recommend Jasmine (http://pivotal.github.io/jasmine/) as they have a fairly clean syntax and runs in both Node and browser environments. Let me know if you need a hand setting that up though I don't have that much time at the moment.
Good feedback. I'm still getting up to speed on Jasmine (and, alternatively, Mocha) for the day job. In the mean time, to address this bug I've cranked out an OKJS test (something home grown https://github.com/gkindel/okjs ) which will work to provide basic coverage which I'll check in with the patch.
This library was written as a weekend exercise, but I'm happy that people are finding it useful. Would it be valuable to do the whole the whole npm-gruntjs-requirejs-amd-kharma-jasmine-fu thing on it?
One day it could be as awesome as http://html9responsiveboilerstrapjs.com/ ;)