search

dominictarr / split

MIT License
347 stars 39 forks source link

Fix splitting empty string chunks #14

Closed tjmehta closed 9 years ago

tjmehta commented 9 years ago

I noticed when using es.replace to remove whitespace using multiple RegExps, that if a split resulted in empty string chunks 'undefined' was being inserted into the stream. I will PR a test case and split-version-bump in event-stream for es.replace shortly.

tjmehta commented 9 years ago

apologies for the whitespace trimming messing up the diff (me editor does it automatically)

dominictarr commented 9 years ago

This looks like a good PR but I don't understand correctly what the problem is. Can you give a simple example that shows what input is required to get incorrect output?

tjmehta commented 9 years ago

Hey Dominic,

Check out the second to last commit on the PR branch: it shows the broken test without the fix.

When an empty chunk is split by empty it results in an empty array ('pieces') making 'soFar' 'null'. When the next chunk passes through split it is prepended with 'soFar' which is 'undefined'.

—@tjmehta Typed using my thumbs..

On Sun, Feb 1, 2015 at 5:25 PM, Dominic Tarr notifications@github.com wrote:

This looks like a good PR but I don't understand correctly what the problem is.

Can you give a simple example that shows what input is required to get incorrect output?

Reply to this email directly or view it on GitHub: https://github.com/dominictarr/split/pull/14#issuecomment-72395845

dominictarr commented 9 years ago

thanks, I saw the event-stream issue after this one and merged it.

On Mon, Feb 2, 2015 at 8:17 PM, Tejesh Mehta notifications@github.com wrote:

Hey Dominic,

Check out the second to last commit on the PR branch: it shows the broken test without the fix.

When an empty chunk is split by empty it results in an empty array ('pieces') making 'soFar' 'null'. When the next chunk passes through split it is prepended with 'soFar' which is 'undefined'.

—@tjmehta Typed using my thumbs..

On Sun, Feb 1, 2015 at 5:25 PM, Dominic Tarr notifications@github.com wrote:

This looks like a good PR but I don't understand correctly what the problem is. Can you give a simple example that shows what input is required to get

incorrect output?

Reply to this email directly or view it on GitHub: https://github.com/dominictarr/split/pull/14#issuecomment-72395845

— Reply to this email directly or view it on GitHub https://github.com/dominictarr/split/pull/14#issuecomment-72415072.

dominictarr commented 9 years ago

merged into 0.3.3

On Mon, Feb 2, 2015 at 11:09 PM, Dominic Tarr dominic.tarr@gmail.com wrote:

thanks, I saw the event-stream issue after this one and merged it.

On Mon, Feb 2, 2015 at 8:17 PM, Tejesh Mehta notifications@github.com wrote:

Hey Dominic,

Check out the second to last commit on the PR branch: it shows the broken test without the fix.

When an empty chunk is split by empty it results in an empty array ('pieces') making 'soFar' 'null'. When the next chunk passes through split it is prepended with 'soFar' which is 'undefined'.

—@tjmehta Typed using my thumbs..

On Sun, Feb 1, 2015 at 5:25 PM, Dominic Tarr notifications@github.com wrote:

This looks like a good PR but I don't understand correctly what the problem is. Can you give a simple example that shows what input is required to get

incorrect output?

Reply to this email directly or view it on GitHub: https://github.com/dominictarr/split/pull/14#issuecomment-72395845

— Reply to this email directly or view it on GitHub https://github.com/dominictarr/split/pull/14#issuecomment-72415072.