Implement quote_keys flag for std functions manifestYamlDoc/manifestY…#143
Implement quote_keys flag for std functions manifestYamlDoc/manifestY…#143jnyi wants to merge 1 commit intodatabricks:masterfrom
Conversation
…amlStream to match official jsonnet doc
lihaoyi-databricks
requested review from
szeiger
and removed request for
lihaoyi-databricks
|
Passing the review to @szeiger |
|
|
||
| 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.
|
I cloned your PR and finished the work in #231 |
Forked from PR #143 --------- Co-authored-by: Yi Jin <yi.jin@databricks.com>
4 participants
…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