Implement quote_keys flag for std functions manifestYamlDoc/manifestY… #143
Conversation
…amlStream to match official jsonnet doc
lihaoyi-databricks
requested review from
szeiger
and removed request for
lihaoyi-databricks
|
Passing the review to @szeiger |
| @@ -30,6 +30,15 @@ class YamlRenderer(_out: StringWriter = new java.io.StringWriter(), indentArrayI | |||
| } | |||
| } | |||
|
|
|||
| private[this] def removeQuoteKey(): Unit = { | |||
| // refer to quote_keys parameter in https://jsonnet.org/ref/stdlib.html, only unquote keys | |||
| if (!quoteKeys && !afterKey) { | |||
There was a problem hiding this comment.
This looks wrong. quote_keys=false is supposed to omit the quotes if possible, so there should be a check that determines which strings are allowed to be unquoted.
Patching the buffer after writing is a bit awkward. Can we also move this code into writeString so we can determine quoting first (according to the correct criteria, not just the flag) and then emit a quoted or non-quoted string? The new code for non-quoted writing should only end up in a single path because the other ones won't qualify for quote removal anyway.
There was a problem hiding this comment.
I've tested the behavior of jsonnet's quote_keys=false which removes quotes of the yaml keys, so my understanding !afterKeys indicates we are dealing with key right now, as for patching the buffer, agree it is a bit counter-intuitive, let me see if I can do something to writeString, found it is a bit harder to modify that function interfaces which touches more places.
There was a problem hiding this comment.
It should only have to touch the last branch in writeString. There other 2 should not be applicable for quote removal.
…amlStream to match official jsonnet doc
Purpose
This PR creates a flag documented in jsonnet official site std library: https://jsonnet.org/ref/stdlib.html for function
manifestYamlDocandmanifestYamlStream, it will allow us to generate yaml without quoting the keys.Test
./mill -i show "sjsonnet[2.13.4]".jvm.assemblyand run again a locally created test.jsonnet to output YAML via flag--yaml-out