golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.03k stars 17.67k forks source link

text/template: add support for optional chaining #43608

Open thernstig opened 3 years ago

thernstig commented 3 years ago

It would be beautiful if the package text/template (used by e.g. Helm) could support optional chaining. Similar to JavaScript in latest standard ECMAScript 2020. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

It would basically look like this:

spec:
  type: {{ .Values.service?.type | default "ClusterIP" }}

If service is not defined, it would short-circuit to undefined/false and thus default to "ClusterIP".

This could be used in other cases, such as:

spec:
  {{- if .Values.global?.internalIPFamily }}
  ipFamily: {{ .Values.global.internalIPFamily }}
  {{- end }}

It solves a few things in e.g. Helm that uses text/template for templating:

  1. default not working when parent values does not exist in a chain, giving errors, see https://github.com/helm/helm/issues/8026.
  2. It would be able to get rid of the, in my opinion, not great best practice for nested or flat values for e.g. Helm (which I assume is applicable to other text/template usages as well), see https://helm.sh/docs/chart_best_practices/values/#flat-or-nested-values. Optional chaining would instead still make the code more readable (one-liner) not caring about value depth. It would also in my opinion make it easier to read YAML files with the nested structure, clearly separating levels, than to use camelCase.

As I know very little about Go and the packge text/template, feel free to add more precise info on how this translates to what needs to be done in text/template.

cagedmantis commented 3 years ago

This seems more like a proposal. I'm adding the text/template owners in order to begin the discussion.

/cc @robpike @mvdan

bilak commented 3 years ago

@cagedmantis @robphoenix @mvdan please any update on this task? It would be super helpful if we can use this style of property resolver.

thernstig commented 3 years ago

@bilak @cagedmantis @robphoenix @mvdan do you have some opinions on this?

mvdan commented 3 years ago

There are dozens of open text/template issues. Please don't ping proposals every few weeks; we have limited time.

Personally, I think we should implement the already-accepted proposals (https://github.com/golang/go/issues/20531 and https://github.com/golang/go/issues/31103) before we consider new ones. Any help with those is welcome.

thernstig commented 3 years ago

It was 5 months ago this was written, and I find it common courtesy to give a short comment that it has been seen. But each repo to his own.

emmaLP commented 2 years ago

Any update on this please

ianlancetaylor commented 2 years ago

The way to make progress on this is to turn it into a proposal with a precise definition of the suggested change and the new semantics.

For example, if I pass

var s struct {
    p *struct {
        f int
    }
}

to a template and write s.p.f then assuming that s.p is not nil I will get an integer. What should happen for s.p.?f if p is nil? Should I get 0? If I get nil that may cause a different error.

The initial comment on the issue says "feel free to add more precise info on how this translates to what needs to be done in text/template" but that is hard for us to do. There are many many many more people suggesting changes than there are people implementing those changes. This can only scale if the people interested in the change can describe exactly what how it should work.

Thanks.

thernstig commented 2 years ago

@ianlancetaylor I do not have the knowledge in Go to give specifics, but regarding semantics one can look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining which can serve as the spec for how it should work.

But regarding s.p?.f it should return nil (since undefined does not exist in Go?).

Anyone more comfortable in Go; feel free to write up a proper proposal. I can link it in the original issue.

ungerik commented 2 years ago

There is no value in nil pointer panics and maybe not much code that depends on it to work properly. Because it just breaks things and doesn't fix anything, the nicest solution would be to make optional chaining the default.

Now I know we can't have nice things because of the remote chance that some code depends on the panics, backward compatibility and all, so the next best thing would be to make it a configuration option on template level.