Support for utf8 chars

[mcollina]

Fixes #6.

However, I am not happy with this solution, because it depends on https://www.npmjs.com/package/utf-8-validate.

This is a highly popular module, so possibly we should look for a different solution that is JS-only, possibly the same approach that is used by https://github.com/mafintosh/csv-parser, which is very fast.

@mafintosh @maxogden I might need some of your help here

[mcollina]

Removed utf-8-validate, as not really needed.

[mcollina]

Wow, this is already 10% faster than the previous implementation.

The main difference here is that we depend on bl.

Anybody sees a problem with that?

[mcollina]

@heroboy please test.

[mafintosh]

won't this always fire? [] is still thruthy.

[mcollina]

you are right, good spot. Removed.

[heroboy]

Thanks. It works.
I have two suggestions to improve performance.

1. Use https://nodejs.org/api/string_decoder.html to convert chunks to string, it will remember the last unused bytes. Then you don't need to store the buffer list.
2. add {decodeStrings:false} in through options,then if code like:

var s = split();
s.write("aaaaa");

the strings will not be converted to Buffer,and you convert it back again. But you need check the type of chunk.

[mcollina]

1. no you would need to store things, as that remember only the last part of a multibyte utf8 string, not the full line
2. possibly, but then I won't be able to use bl.

Why don't you send a PR, and we compare the perf?

[heroboy]

The second line is broken.

var s = split();
s.on("data", data=> { 
    console.log(data);
})

var str = "烫烫烫\r\n烫烫烫";
var buf = new Buffer(str, "utf8");
for (var i = 0; i < buf.length;i+=2)
{
    s.write(buf.slice(i, i + 2));
}
s.end();

[heroboy]

1. I don't know how to use git.
2. Version control system like git that need downloads all history , the usage experience is not good in China's network.
   But I will try to make my version and compare the perf.

[mcollina]

Please try this one, it's based on StringDecoder, it should be good, and performance is equivalent as the current version.

[heroboy]

I think this one is right.
This is my bench result.
for small file "package.json"

benchSplit*10000: 1394.734ms
benchOldSplit*10000: 1327.251ms
benchThrough*10000: 1062.809ms
benchSplit*10000: 1125.691ms
benchOldSplit*10000: 1156.593ms
benchThrough*10000: 985.630ms

for large file(about 3MB)

benchSplit*100: 3900.208ms
benchOldSplit*100: 4079.545ms
benchThrough*100: 200.518ms
benchSplit*100: 3863.971ms
benchOldSplit*100: 3941.405ms
benchThrough*100: 195.780ms

So I think large file bench is better, it make big difference between through() and split().

[mcollina]

what does benchThrough?

[heroboy]

Just replace split() with require('through2')()

[mcollina]

awesome, are you ok to merge then?

[heroboy]

Very ok.

[mcollina]

@mafintosh any other opinion on this one?

[mafintosh]

toString() is not needed when we use a string decoder

[mcollina]

done :)

[mafintosh]

@mcollina other than my minor nitpick comment i'm 👍

[heroboy]

I think flush should be this:

function flush(cb) {
  var str = this._last + this._decoder.end();
  if (str )
    push(this, this.mapper(str))

  cb()
}

[yoshuawuyts]

@heroboy

Version control system like git that need downloads all history

For future reference: git allows you to specify a --depth flag when cloning which greatly improves download speeds. Cheers! ✨

$ git clone --depth 1 git@github.com:mcollina/split2.git

[mcollina]

@heroboy I've tried to have a test case where that flush is needed, but I can't come up with anything.

Also doing var str = this._last + this._decoder.end(); is going to return a broken utf8 string.

[heroboy]

I'm not very clear about the problem. If the input buffer is not a valid utf8 string, the decoder.end() will use last unused bytes to form an incorrect character, is this needed? And maybe future StringDecoder will cache bytes and emit strings later?

[mcollina]

flush is called after everything coming has been processed, so it means that an incomplete utf8 char has been written, and probably the stream has been cut. If there is anything left in the StringDecoder, probably it's better to emit an error, or just skip it.

@mafintosh any opinions on this one?

[heroboy]

through({encoding:"utf8"}) this one doesn't emit an error.

var s = through({encoding:"utf8"});
s.on("data", data=> { 
    console.log("data",data);
})

var str = "烫烫烫\r\n烫烫烫";
var buf = new Buffer(str, "utf8");
for (var i = 0; i < buf.length-1;i+=1)
{
    s.write(buf.slice(i, i + 1));
}
s.end();

[mcollina]

@heroboy I've just pushed a test that reproduce the problem I'm speaking about:

https://github.com/mcollina/split2/blob/utf-8-support/test.js#L270-L285

This writes a truncated utf-8 char. If that char is not completed, it does not push anything down regarding that char.

[heroboy]

But in other node module, if the input utf8 buffer is not completed,it will output the last "incorrect" chars.

var fs = require("fs");
var buf = new Buffer("烫烫", "utf8");
fs.writeFileSync("broken.txt", buf.slice(0, buf.length - 1));
var s = fs.readFileSync("broken.txt", "utf8");
console.log(s);// output is "烫��"

[mcollina]

Done, let me know @heroboy.

[heroboy]

Great. I think there are no problems. And thank you very much make effort to fix this issue.