search

carvel-dev / ytt

YAML templating tool that works on YAML structure instead of text
https://carvel.dev/ytt
Apache License 2.0
1.68k stars 137 forks source link

Add an optional boolean to not escape html during json encoding #891

Closed WnP closed 8 months ago

WnP commented 9 months ago

Fix #890

prembhaskal commented 9 months ago

checking. one comment about boolean flag, can we also explore if we can pass additional parameters to the json.encode() method. (just a thought I am not sure if it is feasible to do).

Okhoshi commented 9 months ago

one comment about boolean flag, can we also explore if we can pass additional parameters to the json.encode() method. (just a thought I am not sure if it is feasible to do).

There's no other option on the json Encoder, only SetIndent and SetEscapeHTML.

WnP commented 9 months ago

I've fixed the tests and created one for the new feature.

prembhaskal commented 9 months ago

one comment about boolean flag, can we also explore if we can pass additional parameters to the json.encode() method. (just a thought I am not sure if it is feasible to do).

There's no other option on the json Encoder, only SetIndent and SetEscapeHTML.

ignore my previous comment, I think I had a brain fade moment there.

I feel it is better to name the variable as escape_html with the default value as true, which will be inline with what json encoder is doing.

someone intending to not escape html can set it as escape_html=false

prembhaskal commented 9 months ago

@cppforlife @100mik please have a look at this when you get time.

prembhaskal commented 9 months ago

@WnP also could you please check the lint failure when you get time. meanwhile I will try to get our approvers eyes on the review.

WnP commented 9 months ago

Sorry for the delay @prembhaskal, I was quite busy during the last days.

Feel free to ping me if there's something missing.

prembhaskal commented 9 months ago

LGTM, @cppforlife please review when you get time.

sethiyash commented 8 months ago

LGTM!