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 ZSink.splitLines #1384
Add ZSink.splitLines #1384
Conversation
cea6b87
to
e906d76
Compare
Oh this has a bug when |
e906d76
to
8de3602
Compare
All done. @vasilmkd could you have a look? |
It'll be my pleasure. I'll review it first thing in the morning. |
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.
Looks useful! Could be also useful to generalize splitOn(',')
or whatever.
/** | ||
* Splits strings on newlines. Handles both `\r\n` and `\n`. | ||
*/ | ||
final val splitLines: ZSink[Any, Nothing, String, String, Chunk[String]] = |
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.
Not to nitpick here, but I see this as a deviation from the previous sinks, which have all been defined with def
. AFAIK val
creates objects eagerly, and they stay in memory for the duration of the program which references this code. Is that something desirable in this case? I may be completely off base here, I just want to hear the reasoning. Same for every other val
, including the ones in Sink
.
*/ | ||
final val splitLines: ZSink[Any, Nothing, String, String, Chunk[String]] = | ||
new SinkPure[Nothing, String, String, Chunk[String]] { | ||
type State = (Chunk[String], Option[String], Boolean) |
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.
On an urelated note, this is probably what the stepless sinks' code will look like after the refactoring. :)
Or a predicate function. |
This needs a rebase and should be good for merging. |
52a3859
to
a7f7153
Compare
a7f7153
to
bc4a90b
Compare
No description provided.