Skip to content

Commit

Permalink
Fix pkrumins#11: .lines bug creating "0"s from empty lines
Browse files Browse the repository at this point in the history
Empty lines (lines ending with newline immediately) were generating
bogus "0"s as discussed in pkrumins#11.

The problem was caused by a careless use of + in `mergeBuffers`, which
resulted in creating a Buffer with string "0" instead of an empty one
with length 0 when an empty buffer was among the ones being merged.
mergeBuffers also had an undefined case for empty set of input buffers,
which has been fixed to always return a buffer with length 0.

This also makes the .lines getter code more readable by using a better
variable name and eliminating tabs from white space.
  • Loading branch information
netj committed Aug 11, 2013
1 parent 039e7d3 commit 3070204
Showing 1 changed file with 20 additions and 20 deletions.
40 changes: 20 additions & 20 deletions lazy.js
Expand Up @@ -205,34 +205,33 @@ function Lazy(em, opts) {
// Streams that use this should emit strings or buffers only
self.__defineGetter__('lines', function () {
return self.bucket([], function (chunkArray, chunk) {
var newline = '\n'.charCodeAt(0), lastNewLineIndex = 0;
var newline = '\n'.charCodeAt(0), chunkBeginning = 0;
if (typeof chunk === 'string') chunk = new Buffer(chunk);
if (chunk){
for (var i = 0; i < chunk.length; i++) {
if (chunk[i] === newline) {
// If we have content from the current chunk to append to our buffers, do it.
if (i > 0) {
chunkArray.push(chunk.slice(lastNewLineIndex, i));
}

// Wrap all our buffers and emit it.
this(mergeBuffers(chunkArray));
lastNewLineIndex = i + 1;
}
if (chunk) {
for (var i = 0; i < chunk.length; i++) {
if (chunk[i] === newline) {
// If we have content from the current chunk to append to our buffers, do it.
if (i > 0) {
chunkArray.push(chunk.slice(chunkBeginning, i));
}
// Wrap all our buffers and emit it.
this(mergeBuffers(chunkArray));
chunkBeginning = i+1;
}
}
}

if (lastNewLineIndex > 0) {
if (chunkBeginning > 0) {
// New line found in the chunk, push the remaining part of the buffer.
if (lastNewLineIndex < chunk.length) {
chunkArray.push(chunk.slice(lastNewLineIndex));
if (chunkBeginning < chunk.length) {
chunkArray.push(chunk.slice(chunkBeginning));
}
} else {
// No new line found, push the whole buffer.
if (chunk && chunk.length) {
if (chunk.length) {
chunkArray.push(chunk);
}
}
}
return chunkArray;
});
});
Expand Down Expand Up @@ -326,14 +325,15 @@ Lazy.range = function () {
return lazy;
}

var emptyBuffer = new Buffer(0);
var mergeBuffers = function mergeBuffers(buffers) {
// We expect buffers to be a non-empty Array
if (!buffers || !Array.isArray(buffers) || !buffers.length) return;
if (!buffers || !Array.isArray(buffers) || !buffers.length) return emptyBuffer;

var finalBufferLength, finalBuffer, currentBuffer, currentSize = 0;

// Sum all the buffers lengths
finalBufferLength = buffers.reduce(function(left, right) { return (left.length||left) + (right.length||right); }, 0);
finalBufferLength = buffers.reduce(function(left, right) { return left.length + right.length; });
finalBuffer = new Buffer(finalBufferLength);
while(buffers.length) {
currentBuffer = buffers.shift();
Expand Down

0 comments on commit 3070204

Please sign in to comment.