JSON can be encoded to multi-line string with configurable indentation

[tylerschultz]

Describe the problem/challenge you have

I have a yaml value that is a a pretty print, multi-line, JSON string. When I decode that JSON value, modify the json struct, then encode it back to a string, it loses it's multi-line pretty print and is one lined. This makes it visually difficult to compare to the original JSON string.

---
baz: |-
  {
      "foo":"bar"
  }

Describe the solution you'd like

I'd like the JSON module to include a way to pretty print. For example it could be json.encode_pretty(json_struct) or add a named argument to encode, like this: json.encode(json_struct, indent=4). The resulting encoded yaml and JSON would look something like:

---
baz: |-
  {
      "foo": "bar",
      "modified": true
  }

• Kubernetes Slack #carvel@2021-05-25T10:58PT

---

(jsr) edit: included link to originating Slack convo

---

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

[pivotaljohn]

API is a good starting point.

I'm diggin' the opt-in nature of this signature @tylerschultz suggested:

json.encode(json_struct, indent=4)

"pretty" can be in the eye of the formatter, so being more specific is also appealing to me. As is the ability to specify the actual indent size.

Would indent=0 mean JSON on a single-line (i.e. equivalent to not specified)? or multiple lines with no indentation (specifically flat)?

[pivotaljohn]

This issue points to a known need, extends the ytt library in a rather standard way, and articulates a generalized solution ==> "carvel accepted"

[tylerschultz]

I'm interested in working on a PR. LMK if there's anything to discuss.

[pivotaljohn]

I'm interested in working on a PR. LMK if there's anything to discuss.

Fantastic! Thanks for raising your hand, @tylerschultz.

"Acceptance Criteria" that come to mind:

• callers who omit the indent kwarg should continue to get the exact same behavior (encoded to a single line string)
• let indent=0 yield a multi-line encoding with no indentation.
• it's an error to supply an unknown kwarg (helps avoid subtle errors like misspelled kwarg name)
• let's mitigate CWE-409: set an indentation limit to ... 4? (any reason to support bigger than that?)

[pivotaljohn]

Above criteria sets a reasonably defined scope ==> "ready for dev"

[pivotaljohn]

@tylerschultz
Please do let us know if you do pick this up, so we can mark this "story" as "in progress".

Regarding PR modes:

• we interpret "Draft PRs" as an invitation for design-level feedback only.
• we reserve finer-grain and "nit picks" for when a PR is "ready".

[mcwumbly]

Some longwinded support for your proposed acceptance criteria:

let indent=0 yield a multi-line encoding with no indentation

👍 fwiw, this is what in python's, json.dumps does. (JavaScript's JSON.stringify does not multi-line when "space" is 0). In any case, this seems like an edge case that will be used rarely.

let's mitigate CWE-409: set an indentation limit to ... 4? (any reason to support bigger than that?)

I don't see any effective limit in json.dumps, whereas JSON.stringify caps the limit at 10 characters (which could be spaces or anything else). I imagine 2 or 4 spaces being typical. 4 seems like an OK max to start with. I bet we'll never hear a request for more, and if we're wrong, we could always re-evalute and bump it up then...

Out of curiosity, how did you identify that CWE when thinking about the acceptance criteria here?

[tylerschultz]

@pivotaljohn Please consider this PR picked up.

[pivotaljohn]

let indent=0 yield a multi-line encoding with no indentation

👍 fwiw, this is what in python's, json.dumps does. (JavaScript's JSON.stringify does not multi-line when "space" is 0). In any case, this seems like an edge case that will be used rarely.

That's a perfect source of inspiration, here, given Starlark's lineage. Thank you. And that read is on the money.

let's mitigate CWE-409: set an indentation limit to ... 4? (any reason to support bigger than that?)

I don't see any effective limit in json.dumps, whereas JSON.stringify caps the limit at 10 characters (which could be spaces or anything else). I imagine 2 or 4 spaces being typical. 4 seems like an OK max to start with. I bet we'll never hear a request for more, and if we're wrong, we could always re-evalute and bump it up then...

Yup, I was thinking along the same lines. Like, what exactly are you doing if you need more than 4 spaces for indentation? I don't mean to sound snarky, if it's legit, I'd love to know!

Out of curiosity, how did you identify that CWE when thinking about the acceptance criteria here?

I can tell you exactly... lemme look that up in my zettelkasten.... right...

So, about a month ago I became aware of this bug: kubernetes/kubernetes#89535. Which had attached to it this CVE: https://nvd.nist.gov/vuln/detail/CVE-2019-11254 in which you'll see a "Weakness Enumeration" section. Thar be rabbit-holes.

[pivotaljohn]

@tylerschultz + @mcwumbly, re: documentation.

Yes, please:

1. remove the TODOs from your PR to carvel-ytt. 👍🏻 And feel free to use this, here, issue to discuss scope, tasks, ... anything around getting to "done".
2. make a PR into https://github.com/vmware-tanzu/carvel.dev/pulls with your update. For now, simply including a few more examples is sufficient — enough to let readers know the feature exists.

[pivotaljohn]

make a PR into https://github.com/vmware-tanzu/carvel.dev/pulls with your update. For now, simply including a few more examples is sufficient — enough to let readers know the feature exists.

Created requested PR into docs. 👍🏻

[aaronshurley]

I believe that this issue can be closed out now.

[aaronshurley]

I believe that this issue can be closed out now.