[template] removes hard-coded missing key error #8128
Conversation
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| p = strings.render_template(template_string, template_vars) | ||
| want_error_code: eval_builtin_error | ||
| want_result: | ||
| - x: <no value> |
There was a problem hiding this comment.
I don't suppose we could make this <undefined> so that it matches print() and the new string interpolation feature? 🤔
There was a problem hiding this comment.
@srenatus should be doable. It would likely be a string check and replacement, something like:
if err := tmpl.Execute(&buf, templateVariables); err != nil {
return err
}
res := buf.String()
if res == "<no value>" {
return iter(ast.StringTerm("<undefined>"))
}
return iter(ast.StringTerm(res))There was a problem hiding this comment.
What about templates that contain multiple dereferences? I'm sorry, but I'm mostly in the $"...{ }..." string interpolation headspace these days (see #8129).
Long story short, I believe we'll need strings.ReplaceAll(res, "<no value>", "<undefined>"). It's a bit hacky (what if the template happens to contain a verbatim <no value>?), but I think it's OK for now. Better than the error we had before.
strings.render_template(`foo: {{ .bar }} {{ .bar }}`, {"bar": 100}) => "foo: 100 100"
this would need to be
strings.render_template(`foo: {{ .bar }} {{ .bar }}`, {}) => "foo: <undefined> <undefined>"
and I think we should perhaps have a test case for that.
| - | | ||
| package test | ||
| template_string = `{{.testvarnotprovided}}` |
There was a problem hiding this comment.
We should probably also have a case like
| template_string = `{{.testvarnotprovided}}` | |
| template_string = `var: [{{.testvarnotprovided}}]` |
just to be sure we also deal with the template, not just return a <no value> (or <undefined>)
Signed-off-by: Colin Lacy <colinjlacy@gmail.com>
8b1f766 to
fec01b2
Compare
Why the changes in this PR are needed?
Current implementation of
render_templatepasses in an explicitmissingkey=erroroption when rendering a Golang template. That's not the default behavior for Golang, so it's an unexpected result. See #7931 for more detail.What are the changes in this PR?
This PR removes the hard-coded
missingkey=erroroption, giving way to the default template parsing behavior built into Golang.Notes to assist PR review:
See updated tests for example output.
Further comments:
This is admittedly a breaking change. I'm not sure how widely used the
render_templatefunction is, and what the risk is of impacting existing implementations. If the risk is seen as problematic, I'd be happy to close this PR or modify the implementation.