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

Comments

@bep
Copy link
Contributor

@bep 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.

@rsc
Copy link
Contributor

@rsc 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 Oct 18, 2016
@bep
Copy link
Contributor Author

@bep 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 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 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.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.