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

text/template: missingkey=zero ignored for map[string]interface{} #24963

Closed
hookenz opened this issue Apr 20, 2018 · 9 comments
Closed

text/template: missingkey=zero ignored for map[string]interface{} #24963

hookenz opened this issue Apr 20, 2018 · 9 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@hookenz
Copy link

hookenz commented Apr 20, 2018

If the missingkey=zero option is used in text/template expansion, then the result will be <no value> where the input data is interface{} and not string or text.

In the case of a map[string]int, the value of a literal zero will be inserted. This makes sense.
For map[string]string, then an empty string is appropriate.

For map[string]interface{} it puts <no value>. Which in my opinion makes no sense. The template itself is all text. It would be most appropriate to use an empty string if it's not an integer. Or to allow defining what value to put in it's place if the key is missing

What version of Go are you using (go version)?

go version go1.10.1 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Ubuntu 14.04

What did you do?

package main

import (
	"text/template"
	"log"
	"bytes"
	"fmt"
)

func main() {
	data := make(map[string]interface{})
	
	text := "APP_VERSION={{.AppVersion}}"
	tmpl, err := template.New("").Option("missingkey=zero").Parse(text)
	if err != nil {
		log.Fatal(err)
	}
	
	var b bytes.Buffer
	err = tmpl.Execute(&b, data)
	if err != nil {
		fmt.Println("template.Execute failed", err)
	}
	
	fmt.Println("Template text:", text)
	fmt.Printf("Expanded: %+v\n", b.String())
}

What did you expect to see?

Template text: APP_VERSION={{.AppVersion}}
Expanded: APP_VERSION=

What did you see instead?

Template text: APP_VERSION={{.AppVersion}}
Expanded: APP_VERSION=<no value>

@hookenz hookenz changed the title missingkey=zero ignored for map[string]interface{} In text/template expansion missingkey=zero ignored for map[string]interface{} Apr 20, 2018
@ianlancetaylor ianlancetaylor changed the title In text/template expansion missingkey=zero ignored for map[string]interface{} text/template: missingkey=zero ignored for map[string]interface{} Apr 20, 2018
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 20, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Apr 20, 2018
@hookenz
Copy link
Author

hookenz commented Apr 20, 2018

My current workaround. It seems that if treated as an error, the right behaviour ensues.

tmpl, err := template.New("").Option("missingkey=error").Parse(text)
// supress/ignore error if it contains text "map has no entry for key" as missingkey=zero doesn't work currently on map[string]interface{}
if (err != nil && !strings.Contains(err.Error(), "map has no entry for key")) {
  /// normal error path.
}

@mvdan mvdan self-assigned this Apr 20, 2018
@mvdan
Copy link
Member

mvdan commented Apr 22, 2018

I've been poking around a bit. The good news is that this edge case doesn't have any tests, so I don't think that the current behavior is locked into place. The only missingkey option tests use a map[string]int as the data type.

However, I am not convinced whether this is a good idea. I can understand that the code you provided behaves in a somewhat confusing way, but I imagine that forcing missingkey=zero to use empty strings as the zero value for interface{} types would also be confusing.

Take, for example, pipes. In those, a value is passed from one command to the next. In that scenario, passing untyped nil and "" are two very different things, and the former is the technically correct option when one is asking for the zero value of an empty interface type.

So I suspect that applying your change could break pieces of code with missingkey=zero, such as {{ .MissingIface | printf "%v" }}, as it would go from printing <nil> to nothing at all.

Something else that comes to mind is that you could use a workaround for this very specific edge case - that is, the edge case of needing zero values to be "stringified". I have written a simple example here: https://play.golang.org/p/f6hs0UiLX-I

$ go run f.go
<no value> -

Note that the example fails on 1.10 due to a bug with passing nil values to template funcs. It is fixed in master and will ship with 1.11, though: #18716

In summary, I don't think there's anything we can do here. Paging @robpike @rsc @ianlancetaylor to see what others think.

@hookenz
Copy link
Author

hookenz commented Apr 22, 2018

How about providing an option to allow the user to define their own behaviour through a function call?
That call would ultimately return a string which is substituted for the {{.Field}} text.

This would allow non-zero result substitution etc.

@mvdan
Copy link
Member

mvdan commented Apr 23, 2018

You mean an alternative to .Option("missingkey=zero")? Can you clarify how exactly that would work?

Such a suggestion should be a proposal; see https://github.com/golang/proposal.

@mvdan mvdan added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 5, 2018
@hookenz
Copy link
Author

hookenz commented May 22, 2018

I think that decision is best left up to the designer of the template engine. To me, it seems like an obvious feature that is missing which is to set a default value when a key is missing and not just substitute a zero value. This is done with flags in the case where a command line option is not provided there is the ability to set a default value.

@mvdan
Copy link
Member

mvdan commented May 25, 2018

Well, I'm precisely asking because I don't see an addition to text/template that would be simple and not interfere with the other existing options, such as missingkey=.

The only thing that potentially comes to mind is missingkey=emptystring or missingkey=empty, documented like:

The operation returns an empty string.

That would fix your initial example, I believe.

/cc @robpike, who is likely very opinionated about text/template's design.

@rsc
Copy link
Contributor

rsc commented Jun 25, 2018

missingkey=zero for an interface{} is a nil interface{}, which formats as <no value>. Sorry, but that's how the zero formats. The option is doing what it's documented to do.

@rsc rsc closed this as completed Jun 25, 2018
@mvdan
Copy link
Member

mvdan commented Jun 25, 2018

@rsc any thoughts on my missingkey=empty idea above? It does seem to fit with text/template in my mind, although I'm not sure how badly it is needed.

@hookenz
Copy link
Author

hookenz commented Jun 27, 2018

@rsc - Personally I think setting the output as "" for a nil interface{} which eventually ends up as a piece of text as the result is not a good representation of no value. But an empty string "" (empty string) is.

But since you guys don't want to change it, people will forever be having to code around this.

mumoshu added a commit to roboll/helmfile that referenced this issue Jun 4, 2019
…templates

Seems like we are affected by golang/go#24963. That is, even though we internally use the template option `missingkey=zero`, in some cases it still prints `<no value>` instead of zero values, which has been confusing the state yaml parsing.

This fixes the issue by naively replacing all the remaining occurrences of `<no value>` in the rendered text, while printing debug logs to ease debugging in the future when there is unexpected side-effects introduced by this native method.

Fixes #553
mumoshu added a commit to roboll/helmfile that referenced this issue Jun 4, 2019
…templates

Seems like we are affected by golang/go#24963. That is, even though we internally use the template option `missingkey=zero`, in some cases it still prints `<no value>` instead of zero values, which has been confusing the state yaml parsing.

This fixes the issue by naively replacing all the remaining occurrences of `<no value>` in the rendered text, while printing debug logs to ease debugging in the future when there is unexpected side-effects introduced by this native method.

Fixes #553
mumoshu added a commit to roboll/helmfile that referenced this issue Jun 4, 2019
…templates

Seems like we are affected by golang/go#24963. That is, even though we internally use the template option `missingkey=zero`, in some cases it still prints `<no value>` instead of zero values, which has been confusing the state yaml parsing.

This fixes the issue by naively replacing all the remaining occurrences of `<no value>` in the rendered text, while printing debug logs to ease debugging in the future when there is unexpected side-effects introduced by this native method.

Fixes #553
mumoshu added a commit to roboll/helmfile that referenced this issue Jun 4, 2019
…templates (#645)

Seems like we are affected by golang/go#24963. That is, even though we internally use the template option `missingkey=zero`, in some cases it still prints `<no value>` instead of zero values, which has been confusing the state yaml parsing.

This fixes the issue by naively replacing all the remaining occurrences of `<no value>` in the rendered text, while printing debug logs to ease debugging in the future when there is unexpected side-effects introduced by this native method.

Fixes #553
@golang golang locked and limited conversation to collaborators Jun 27, 2019
@rsc rsc unassigned mvdan Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants