Add skipOverflow option #24
Conversation
IronSavior
force-pushed
the
skip_overflow
branch
from
6df1971 to
7036986
Compare
| @@ -103,6 +112,7 @@ function split (matcher, mapper, options) { | |||
| stream.matcher = matcher | |||
| stream.mapper = mapper | |||
| stream.maxLength = options.maxLength | |||
| stream.skipOverflow = options.skipOverflow | |||
There was a problem hiding this comment.
Can you add a stream.overflow property here?
There was a problem hiding this comment.
I will do that in the morning.
index.js
Outdated
| if (this.overflow) { | ||
| var buf = this[kDecoder].write(chunk) | ||
| list = buf.split(this.matcher) | ||
| if (list.length === 1) return cb() |
There was a problem hiding this comment.
I don't understand this return cb()
There was a problem hiding this comment.
String#split() returns an array containing one element in exactly one case. If the length of list is 1 here, then it means the decoded chunk did not contain the matcher and we therefore want to ignore this chunk (because it followed an overflow) by returning without any push or buffering.
If we continued to buffer while overflowed, then we pay a substantial cost in terms of memory and time, which would defeat the purpose of adding this new option.
There was a problem hiding this comment.
Can you put a comment explaining all of this?
IronSavior
force-pushed
the
skip_overflow
branch
2 times, most recently
from
6939cb2 to
845b074
Compare
|
Does that make it clear? I generally like to keep explain-the-code comments to a minimum. Also, that style linter is cranky! |
Also addresses surprising maxLength behavior described in mcollina#23
IronSavior
force-pushed
the
skip_overflow
branch
from
845b074 to
618c73d
Compare
Also addresses surprising maxLength behavior described in #23