Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add lazy dropEnd and friends #395
Add lazy dropEnd and friends #395
Changes from 11 commits
d2c8c32
9421dcd
891f82f
c0607a3
ad00a22
a198e80
cd822ae
1ae4ab7
b688a25
413d3f1
c48d764
909e02b
4240db9
eedf0a2
7dc6214
6911faf
af2f32e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for using a lazy
foldl
here instead offoldl'
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lazy fold here is necessary so that we don't force
go deque
cs`. However this has now been changed (see above review comment)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of operating on lists of strict bytestrings, it seems convenient to simply use the lazy bytestring type. Is there a reason not to do that here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, seems nicer. The only problem is that there is no lazy left fold for chunks, and so we need to first reverse chunks and fold from the right. Might be even better to pay traveral than have the space leaked by the foldl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An example would be nice to have here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used
Chunk (pack [1,2]) (Chunk (pack [3,4,6])) Empty
as an example of a lazy bytestring. Hope it's not too verbose, but textual representation of a bytestring is always tricky.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that would be helpful for users who will mostly not be aware of
ByteString
's internal constructors. If you want to represent the bytes as numbers, you can use theOverloadedLists
syntax, e.g.[1,2,3,4,6]
.