search

ben-strasser / fast-cpp-csv-parser

fast-cpp-csv-parser
BSD 3-Clause "New" or "Revised" License
2.11k stars 440 forks source link

need support for CR as the line delimiter #64

Closed stevenklassen8376 closed 6 years ago

stevenklassen8376 commented 6 years ago

I'm not sure where these files came from (this was the standard for Mac OS9 but I know they are not that old). But I have to be able to deal with them.

stevenklassen8376 commented 6 years ago

I have a fix ready to push if you want to grant me permission.

ben-strasser commented 6 years ago

Hi,

thanks for the patch.

Newline endings are very tricky. Consider the case when someone in Linux writes the string "foo\rbar" to a file. If he reads it back in, he does not expect to have generated a line end.

There are already problems of this sort because mixing Windows and Unix newlines is allowed. However, currently I believe that the corner cases are rare enough to be acceptable. I do not think that this trade-off is true for old Apple newlines.

I think the easiest way to deal with such files is piping them trough "tr '\r' '\n'" in the shell.

Best Regards Ben Strasser

On 04/13/2018 12:55 AM, Steven Klassen wrote:

I have a fix ready to push if you want to grant me permission.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ben-strasser/fast-cpp-csv-parser/issues/64#issuecomment-380968809, or mute the thread https://github.com/notifications/unsubscribe-auth/ALaAj4jlOQxq82G0bcUyvDCtrCsVr3Yqks5tn9tIgaJpZM4TSnC-.

stevenklassen8376 commented 6 years ago

Your response isn't quite clear to me. Would you like me to push the patch or not?

As for where my files came from, it turns out they come from some firmware beyond our control. It's not that old so I'm surprised by their developers' choice, but I have to deal with it nonetheless.

ben-strasser commented 6 years ago

I do not want to add the patch to the mainline and gave a reason as to why.

If I was in your situation, I would pipe the output of the firmware through "tr '\r' '\n'" and be done with it.

Best Regards Ben Strasser

On 04/26/2018 08:08 PM, Steven Klassen wrote:

Your response isn't quite clear to me. Would you like me to push the patch or not?

As for where my files came from, it turns out them come from some firmware beyond our control. It's not that old so I'm surprised by their developers' choice, but I have to deal with it nonetheless.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ben-strasser/fast-cpp-csv-parser/issues/64#issuecomment-384736322, or mute the thread https://github.com/notifications/unsubscribe-auth/ALaAj3YxT4RbNV2k7yw51PhXZFn2kJwzks5tsg0lgaJpZM4TSnC-.

stevenklassen8376 commented 6 years ago

Ok, thanks. I don't like the extra piping but I'll keep my patch local. An alternative that I may consider is to assume the newline is "\n" but allow it to be changed in the constructor or something.

But for now you can consider this issue as closed.