Add EOL option.

[btimby]

• Caller can now specify desired EOL sequence: '\r', '\n' or '\r\n'.
• Simplify implementation.

[btimby]

@catdad Thanks for the library. I will mention my use-case. I have an FTP server written for node, and I need to implement ASCII file transfer mode. In that mode, the server must convert from CRLF to LF. Then on download, it must convert from LF to CRLF. The library as it stood allowed for the former, but not the latter.

BTW, I reviewed all the libraries on npm that I could find for this task and they were all broken, overly complex, did not handle streams or did not handle the carryover from a previous chunk.

I may have some additional PRs once I integrate this into my codebase, I am working with file and socket streams there.

[catdad]

Thanks for submitting this PR!

In the description you say that you "simplify implementation", but that doesn't really give me information about what you changed and why you changed it. Can you describe the simplifications you made and why they were made, please?

[btimby]

The simplification I refer to is in two forms.

1. I combined the various regular expressions into one. The regex /\r\n|\n\r|\r|\n/g will match any sequence of line ending chars. In the order specified within the expression. Thus \r\n will match before the individual \r and \n. This eliminates checking for these sequences individually (basically, I let the regex engine do the same thing in one replace step).
2. I modified the way a trailing CRLF is handled on previous chunk. Rather than remembering if it is a CR or LF, I simply carry it over to the next chunk. I use the end callback in through to emit this carryover after the final chunk.

[btimby]

As far as why, I am always looking for ways to remove lines of code, as lines of code have bugs. less code = less bugs.

Not only that, but allowing the caller to choose the replacement EOL char (my requirement) required the logic to be modified. Instead of adding a ton of new conditions, I just simplified the code so that existing EOL chars are all handled in a uniform way.

[catdad]

I updated the build so that it once again is running. Please take a look at the current failures. Some are due to issues I already mentioned in the review.

[btimby]

I thought I had pushed those fixes yesterday when I replied. Either I forgot or it failed for some reason. In any case, they are pushed now.

[catdad]

Sorry for the delay, I got super distracted. I made some additional changes and fixed some additional issues, and this looks good now. Thanks for adding this feature!

[catdad]

Unfortunately, I actually had to revert this. I found out after I merged it that it does have a bug in it and does not allow for the same functionality as the previous implementation.

The bug is in this line:

var endCrlf = /\r|\n$/;

This regular expression checks for a \r anywhere in the string or a \n at the end of the string. This can help illustrate that:

The previous code included behavior when either is at the end. Looking at code coverage, you can see that the else in that check is not covered. When I change the expression to match both being at the end (either /(\r|\n)$/ or /\r$|\n$/), all the tests time out and the stream never completes.

I didn't have time to investigate what the issue is. If you want, feel free to recreate this PR and fix the issue.

I guess if nothing else, we both learned that less code can have more bugs 😉.

[btimby]

I made a new PR. I don't think I learned anything of the sort. The solution was to REMOVE CODE ;-)