Add printToAppendable
#1855
Add printToAppendable
#1855
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1855 +/- ##
==========================================
- Coverage 86.21% 86.01% -0.20%
==========================================
Files 73 73
Lines 2531 2531
Branches 143 143
==========================================
- Hits 2182 2177 -5
- Misses 349 354 +5
Continue to review full report at Codecov.
|
| final def printToAppendable(json: Json, out: Appendable): Unit = { | ||
| val folder = new AppendableFolder(out) | ||
| json.foldWith(folder) | ||
| } | ||
|
|
There was a problem hiding this comment.
Thanks for the PR! I think circe has really tried to avoid public methods with side effecting signatures, so I'm fairly hesitant to introduce one. That being said, I think if we mark the boundaries of safeness and let others know that thar be dragons.. then we can probably let it in. So something like this:
| final def printToAppendable(json: Json, out: Appendable): Unit = { | |
| val folder = new AppendableFolder(out) | |
| json.foldWith(folder) | |
| } | |
| object Unsafe { | |
| final def printToAppendable(json: Json, out: Appendable): Unit = { | |
| val folder = new AppendableFolder(out) | |
| json.foldWith(folder) | |
| } | |
| } |
And we should add some scaladoc to the method as well. For instance, it should be noted that this method returns only after all JSON is written to the appendable and that any use of the Appendable outside this method voids the warranty 😆 .
Yes, that's completely fair! Thanks for being open to it. The goal here is mostly to expose a higher-performance API, so here's an idea to do it safely:
The catch with this is that even though Otherwise... 👍 to Thanks again for the review, and congrats on maintainership :) |
|
I completely understand the goal of the higher-performance API. Lets for now go with I'm thinking that we might need a different implementation completely instead of what we have here... maybe something based on Happy to help and thanks! |
|
Sounds good, updated!
I'm not sure. I was imagining a circe version of this fs2 method: def writeOutputStream[F[_]](
fos: F[OutputStream],
closeAfterUse: Boolean = true
)(implicit F: Sync[F]): Pipe[F, Byte, INothing]So something like this (bikeshed the method names): // this one prints a stream of JSON as an array:
// [{},{},{}]
def printArrayToAppendable[F[_]](
fapp: F[Appendable],
)(implicit F: Sync[F]): Pipe[F, Json, INothing]
// this one uses a custom delimiter instead, e.g.:
// {}\n{}\n{}
def printToAppendable[F[_]](
fapp: F[Appendable],
delim: CharSequence,
)(implicit F: Sync[F]): Pipe[F, Json, INothing] |
|
To bikeshed a bit, i was just imaging something of the signature For your use case in Feral you would be able to then do Reason I suggest a different printer implementation is I think having an FS2 native implementation would be able to suspend the encoding of the json better, and just overall work better with FS2 than trying to hack around |
|
@armanbilge Thanks again for the PR. I'll have @zmccoy give it a review, and we'll get this into the next release. |
Not sure if the precedent here is to avoid side-effecting APIs at all costs (if so, apologies), but personally I would find this useful. I started this PR to add an API for writing to an
OutputStreambut realized it could be more general.