feat: allow multiple documents in std.parseYaml
#205
Conversation
|
Heya @lihaoyi-databricks 👋 Is there any chance I could please get a review on this one? ❤️ Thanks! |
| val yaml: Seq[Object] = new Yaml(new Constructor()).loadAll(yamlString).asScala.toSeq | ||
| yaml match { | ||
| case l if(l.size > 1) => new JSONArray(yaml.asJava).toString() | ||
| case _ => new JSONObject(yaml.iterator.next().asInstanceOf[java.util.LinkedHashMap[String, Object]]).toString() |
There was a problem hiding this comment.
There is no need to call iterator, the seq can access value with seq(1)
This can throw NPE or NoSuchElementException when the yaml is null or empty.
Jsonnet has a ujson, maybe we should use that ?
Test cases？
There was a problem hiding this comment.
Thanks @He-Pin !
I've performed a small refactor of the function; I did explore using ujson instead of the JSONArray and JSONObject types. However, I ran into a bit of trouble trying to serialize an arbitrary Seq into the ujson.Arr or ujson.Obj? types.
Looking around there wasn't much documentation under "Serializing Scala data types into ujson", and my GitHub-fu of taking a look throughout the repo/other opensource projects let me down a little there 😓
If you have any pointers on as to how I can serialize a Seq[Object] into a ujson.Obj or ujson.Arr elegantly then I'd be more than happy to follow up with some more commits, let me know! ❤️
Thank you again for the review also! 🙏
Heya Team 👋
While trying to give
sjsonneta whirl, we noticed a few of ourstd.parseYamlcalls were failing if they included multi doc YAML. (deliminated by the old---).This PR allows the
yamlToJsonfunction to parse these files/YAML streams and return the appropriate JSON string.I'm a first time contributor to this repo, and furthermore it's my first time writing Scala.
Please do let me know if there's a better way to accomplish this, I'm more than happy to work through any of the feedback I receive!
Thanks!