feat: add yaml to/from functions #360
Conversation
In the same vein of Masterminds#223, add YAML conversion functions. Fixes Masterminds#358.
|
any update on this? |
|
@technosophos @mattfarina would you like to have a look at it? Seems to be a piece of cake 🍰 :) |
|
I mistakenly opened a PR that did the same thing. I just closed it, but wanted to chime in to say I could really use these! Thanks for all the work you guys do 🙂 |
| @@ -3,6 +3,7 @@ package sprig | |||
| import ( | |||
| "bytes" | |||
| "encoding/json" | |||
| "gopkg.in/yaml.v3" | |||
There was a problem hiding this comment.
it would be better to use "sigs.k8s.io/yaml" to supoort "omitempty" json tags
There was a problem hiding this comment.
Better is a strong statement, just different.
There was a problem hiding this comment.
Could you elaborate why we would benefit from omitempty in this context?
We can't specify tags during the templating
There was a problem hiding this comment.
AFAICT the Helm version of this doesn't specifically use omitempty either, so not sure whether that is really relevant. It does, however, make sense to use the same parser that Helm does, in case there are other differences in implementation.
(note that Helm uses a different yaml engine for toYamlPretty
|
Appreciate your work on this @blakepettersson, when you have a chance can you review the comments? Would love to see this integrated in! Awesome job! |
| } | ||
|
|
||
| // mustFromYaml decodes YAML into a structured value, returning errors. | ||
| func mustFromYaml(v string) (interface{}, error) { |
There was a problem hiding this comment.
The use of 'must' typically implies that it will panic if the function fails. I understand this is the convention here, but it feels a bit weird.
There was a problem hiding this comment.
Sure is, but as you said I'm just following the convention from elsewhere.
There was a problem hiding this comment.
@blakepettersson Sorry for the slow response. This repo is maintained but we only pass by and act on occasion.
I would be interested in adding this functionality. But, the primary user of sprig is Helm and Helm already has two of the functions. We would want them to be compatible so that we could remove them from Helm and just use the sprig ones. You can find the Helm details at https://github.com/helm/helm/blob/main/pkg/engine/funcs.go.
Can you make them compatible?
|
@mattfarina in the case of Would it make sense to do the same with In the case of |
|
@mattfarina , any chance you could review blacke's comments above? Would like to see this implemented as this would help a lot of downstream projects! Thanks! |
|
1 Year :( |
## Description We continue to backport features, bug fixes, and more from Sprig to Sprout. This time, I'm focusing on the YAML functions part, used by Helm and intended for use by others as well. ## Changes - Add `fromYaml` function to convert yaml string into a go map - Add `toYaml` function to convert any to a yaml string - Add `mustFromYaml` function to convert and return error to the template system when occurs - Add `mustToYaml` function to convert and return error to the template system when occurs ## Fixes Masterminds/sprig#358 Masterminds/sprig#360 ## Checklist - [x] I have read the **CONTRIBUTING.md** document. - [x] My code follows the code style of this project. - [x] I have added tests to cover my changes. - [x] All new and existing tests passed. - [ ] I have updated the documentation accordingly. - [x] This change requires a change to the documentation on the website. ## Additional Information The documentation for the encoding part of the library will be released soon after this pull request on https://sprout.atom.codes Initially implemented by @blakepettersson on sprig
|
Hello everyone 👋, I wanted to let you know that this issue has been addressed in the fork of this project at go-sprout/sprout. The fix has been implemented starting from version For those looking for a solution, I recommend checking out the latest releases of the fork. This should help address the issue discussed in this thread. Thank you! |
In the same vein of #223, add YAML conversion functions. Fixes #358.