Option encoder #296
Option encoder #296
Conversation
| val some = Foo(Some("some")) | ||
| val none = Foo(None) | ||
|
|
||
| assertNoDiff(some.asYaml, "field: some") |
There was a problem hiding this comment.
Do we have a test going the other way? Or printing is not possible at all currently?
| @@ -18,6 +18,11 @@ object YamlEncoder extends YamlEncoderCrossCompanionCompat { | |||
| implicit def forBoolean: YamlEncoder[Boolean] = v => Node.ScalarNode(v.toString) | |||
| implicit def forString: YamlEncoder[String] = v => Node.ScalarNode(v) | |||
|
|
|||
| implicit def forOption[T](implicit encoder: YamlEncoder[T]): YamlEncoder[Option[T]] = { | |||
| case Some(t) => encoder.asNode(t) | |||
| case None => Node.ScalarNode("", Tag.nullTag) | |||
There was a problem hiding this comment.
is there some reason to encode null as scalar node with null tag? I know this should be question to myself, but just thinking loudly
There was a problem hiding this comment.
There are no Null nodes and I didn't check if null can be passed as a value to Scalar node. I wouldn't expect it to be tbh.
There was a problem hiding this comment.
Node.ScalarNode("", Tag.nullTag) seems like a hack to represent something more complex that model was designed for :D It isn't obvious for me, but maybe it is ok because it is how spec defines it - https://yaml.org/spec/1.2.2/#10211-null.
There was a problem hiding this comment.
ehh, I now saw the how Tag looks like. I think it's ok as it is tag which determines how value of scalar should be interpreted
|
@lbialy I think this can be merged? |
|
Wait, tests!
…On Wed 29. May 2024 at 16:01, Tomasz Godzik ***@***.***> wrote:
@lbialy <https://github.com/lbialy> I think this can be merged?
—
Reply to this email directly, view it on GitHub
<#296 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACBVNUWCI7QGRLA2X5W2DFDZEXNUVAVCNFSM6AAAAABHQU6HP6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZXGQ4TGNRQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Supersedes #127 (thanks @kpodsiad), closes #119