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: index is too restrictive in its type validation #15974

Closed
bep opened this issue Jun 6, 2016 · 4 comments
Closed

text/template: index is too restrictive in its type validation #15974

bep opened this issue Jun 6, 2016 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bep
Copy link
Contributor

bep commented Jun 6, 2016

Given:

package main

import (
    "html/template"
    "os"
)

func main() {
    m := make(map[template.HTML]string)
    m["Key"] = "Val"

    // OK!
    if _, ok := m["Key"]; !ok {
        panic("Map lookup failed")
    }

    t, err := template.New("foo").Parse(data)
    if err != nil {
        panic(err)
    }

    // This fails
    err = t.Execute(os.Stdout, m)

    if err != nil {
        panic(err)
    }
}

var data = `{{ index . "Key" }}`

This panics:

panic: template: foo:1:3: executing "foo" at <index . "Key">: error calling index: value has type string; should be template.HTML

Juggling around with template.HTML and string types seems like a common thing in Go templates, and I would expect the index func to work the same way as a straight map lookup in this case.

@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Jun 6, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@rsc
Copy link
Contributor

rsc commented Oct 18, 2016

Just like in #14751, index mirrors ordinary Go execution. It doesn't just take anything and do its best. This is not, say, Javascript.

@rsc rsc closed this as completed Oct 18, 2016
@bep
Copy link
Contributor Author

bep commented Oct 18, 2016

@rsc in this case it does definitely not mirror Go execution.

m["Key"] // Ok!

Is different than:

{{ index . "Key" }} // Panic!

I understand your resistance to change existing behavior in Go, but please don't make up arguments that doesn't hold water.

template.HTML is a string -- and when strings can be used as keys in map[template.HTML] lookups, then the same should be the case for the index func. A template.HTML isn't anything, it is a well defined string.

@cespare
Copy link
Contributor

cespare commented Oct 18, 2016

@bep you wrote

when strings can be used as keys in map[template.HTML] lookups

but they cannot. Only template.HTMLs can be used. In this code:

m["Key"] // Ok!

"Key" is an untyped string constant so it may be realized as a template.HTML when given a type.

In this template code:

{{ index . "Key" }} // Panic!

"Key" is a string. If you wrote

k := "Key"
_ = m["Key"]

you'd get the equivalent compile error: cannot use k (type string) as type "html/template".HTML in map index.

https://play.golang.org/p/wdLJzrtl_p


All that said, I agree that the template code is necessarily more clumsy than Go code because it lacks the convenience of constants. I'm not sure anything can be done to improve the situation in a backwards-compatible manner, though.

@bep
Copy link
Contributor Author

bep commented Oct 18, 2016

Any way you spint it: given my latest examples index func will not win any usability price awards. The "cannot fix because it would break" is an argument I'll happily buy, but I just get slightly annoyed by these this is as designed arguments.

@golang golang locked and limited conversation to collaborators Oct 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants