Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made SplitStreamSequence work across multiple chunks for strings #70

Merged
merged 1 commit into from
Feb 26, 2014

Conversation

FrozenCow
Copy link
Contributor

(Hi again, sorry for the previous pull request, but apparently I still stumbled across a different problem ;) )
In my use-case of Lazy I'm reading a file that needs to be split into large pieces. The chunks that the stream spits out are smaller than the pieces I need. At the moment Lazy.js splits on each chunk separately, which results in Lazy spitting out the chunks as-is, instead of actual splitted pieces.

This also goes for line-splitting. There can be multiple lines inside one chunk. Most of those will be correct, but the lines that span across 2 chunks will be incorrect as far as I've seen.

This patch contains a solution for this by concatenating chunks and looking up the divider inside that concatenation. It also includes a test that fails with the earlier code and passes with the code in this patch.

However, some other tests fail. This is because I have only implemented a solution for string-dividers, not regexp-dividers! I need some hints as to how to solve this for regexp-dividers, since those don't have a constant length. Maybe for regexp dividers it should just fallback to the existing implementation, but that's somewhat confusing as you don't expect the output to be like this. Let me know what you think!

@dtao
Copy link
Owner

dtao commented Feb 23, 2014

Awesome, thanks for working on this! I was aware of this issue (in theory anyway--in fact I think there's a comment about it somewhere in the specs) but just hadn't gotten around to solving it yet.

I am not at a computer right now but I will take a look (and almost certainly merge) soon.

@dtao dtao merged commit 52b2ae0 into dtao:master Feb 26, 2014
@FrozenCow FrozenCow deleted the streamsplit branch February 27, 2014 11:46
@FrozenCow
Copy link
Contributor Author

Thanks for both merges! Nice going on the regexp solution too, looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants