Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fromJson and mustFromJson funcs #223

Merged
merged 1 commit into from Dec 2, 2020
Merged

Add fromJson and mustFromJson funcs #223

merged 1 commit into from Dec 2, 2020

Conversation

mholt
Copy link
Contributor

@mholt mholt commented Jan 4, 2020

Resolves #35; I also found us needing this in Caddy. See caddyserver/caddy#2736 (comment).

Incidentally, my editor made a number of good points that I agree with:

Screen Shot 2020-01-04 at 12 53 41 PM

I don't suppose there's any chance of fixing these... (v4?)

Also, JSON functions should probably documented along with the encoding functions, maybe?

// mustFromJson decodes JSON into a structured value, returning errors.
func mustFromJson(v string) (interface{}, error) {
var output interface{}
err := json.Unmarshal([]byte(v), &output)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will silently fail if v cannot be converted to a []byte. The type conversion should be checked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a compile time check. I believe all strings can be converted to byte slices. (This is not a type assertion.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, you're right. However, I would extend the function to also accept []byte as an input.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data flowing between template functions can be of any type. It's entirely possible that someone will want to use this function with a []byte, for example:

{{ (readFile "foo.json" | fromJson).bar }}

where readFile returns a []byte. Why not allow this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can. I was just wondering what the use case was.

@ggillies
Copy link

ggillies commented Mar 6, 2020

Hi!

I am really keen to see this merged and become usable. Is there anything I can do to help? Is the only thing outstanding here to change input type from string to []byte?

@renjiexu
Copy link

+1, this feature could be very helpful

@paichinger
Copy link

This feature would be incredibly useful. How can we get this done?

@mattfarina
Copy link
Member

@mholt In reference to your editors suggestions... that would change the public API of this library. That would mean a major version increment so they will have to wait until then. When this version of the library was being written Go modules were still in transition. Breaking APIs was a real issue at that time for some so we avoided it back then. This is just the context of why the capitalization is what it is.

@mattfarina mattfarina merged commit 1bbf15f into Masterminds:master Dec 2, 2020
@mholt
Copy link
Contributor Author

mholt commented Dec 2, 2020

That's understandable; thanks for the merge!

blakepettersson added a commit to blakepettersson/sprig that referenced this pull request Feb 7, 2023
In the same vein of Masterminds#223, add YAML conversion functions. Fixes Masterminds#358.
27149chen added a commit to jibutech/sprig that referenced this pull request May 16, 2023
* feat: add yaml to/from functions

In the same vein of Masterminds#223, add YAML conversion functions. Fixes Masterminds#358.

* chore: bump yaml version to v3

---------

Co-authored-by: Blake Pettersson <blake.pettersson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add parseJson
7 participants