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: function chaining #3140

Closed
gopherbot opened this issue Feb 27, 2012 · 12 comments
Closed

text/template: function chaining #3140

gopherbot opened this issue Feb 27, 2012 · 12 comments
Assignees

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 27, 2012

by robfig@yext.com:

What steps will reproduce the problem?
Apply the attached patch to template/exec_test.go

What is the expected output?
All tests pass

What do you see instead?
10:12:52 [robfig@robfig ~/go/src/pkg/template]
$ gomake test
gotest
rm -f _test/template.a
6g  -o _gotest_.6 exec.go funcs.go helper.go parse.go set.go  exec_test.go set_test.go
rm -f _test/template.a
gopack grc _test/template.a _gotest_.6 
--- FAIL: template.TestExecute (0.01 seconds)
    mapString: expected
        "value"
    got
        ""
    mapNiladicFunc: unexpected execute error: template: mapNiladicFunc:1: wrong number of args for mapNiladicFunc: want 0 got 1
    mapVariadicFunc: unexpected execute error: template: mapVariadicFunc:1: wrong number of args for mapVariadicFunc: want 0 got 4
FAIL
gotest: "./6.out" failed: exit status 1
make: *** [test] Error 2

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
OSX

Patch from "hg diff": 

diff -r c1702f36df03 src/pkg/template/exec_test.go
--- a/src/pkg/template/exec_test.go Tue Oct 18 10:55:12 2011 +1100
+++ b/src/pkg/template/exec_test.go Mon Feb 27 10:16:46 2012 -0500
@@ -402,6 +402,11 @@
    {"error method, error", "{{.EPERM true}}", "", tVal, false},
    {"error method, no error", "{{.EPERM false}}", "false", tVal, true},
 
+   // Func chaining.
+   {"mapString", "{{mapString.key}}", "value", tVal, false},
+   {"mapNiladicFunc", "{{mapNiladicFunc.key}}", "value",
tVal, true},
+   {"mapVariadicFunc", "{{mapVariadicFunc.key 1 2 3}}",
"args=[1, 2, 3]", tVal, true},
+
    // Fixed bugs.
    // Must separate dot and receiver; otherwise args are evaluated with dot set to variable.
    {"bug0", "{{range .MSIone}}{{if $.Method1 .}}X{{end}}{{end}}", "X", tVal, true},
@@ -431,6 +436,26 @@
    return "oneArg=" + a
 }
 
+func mapString() map[string]string {
+   return map[string]string{
+       "key": "value",
+   }
+}
+
+func mapNiladicFunc() map[string]func()string {
+   return map[string]func()string {
+       "key": func() string {
+           return "value"
+       },
+   }
+}
+
+func mapVariadicFunc() func(...interface{})string {
+   return func(args ...interface{})string {
+       return fmt.Sprintf("args=%s", args)
+   }
+}
+
 // count returns a channel that will deliver n sequential 1-letter strings starting at "a"
 func count(n int) chan string {
    if n == 0 {
@@ -459,6 +484,9 @@
        "typeOf":   typeOf,
        "vfunc":    vfunc,
        "zeroArgs": zeroArgs,
+       "mapString": mapString,
+       "mapNiladicFunc": mapNiladicFunc,
+       "mapVariadicFunc": mapVariadicFunc,
    }
    for _, test := range execTests {
        tmpl := New(test.name).Funcs(funcs)

Which revision are you using?  (hg identify)


Please provide any additional information below.
@rsc
Copy link
Contributor

@rsc rsc commented Feb 27, 2012

Comment 1:

I think this is working as intended, but assigning to r for a final say.
If you write F.Foo 1 2 3 then there is no way for the template to
distinguish the intents F.Foo(1, 2, 3) - what it does now -
and F(1, 2, 3).Foo - what I think you are asking for.
I don't believe we should add a special case here.

Labels changed: added priority-go1, removed priority-triage.

Owner changed to @robpike.

Status changed to Accepted.

@robfig
Copy link
Contributor

@robfig robfig commented Feb 27, 2012

Comment 2:

Hm, that's not what I'm asking for.  I think it should have the same behavior as chained
methods.  According to the docs, niladic methods can be part of a chain (e.g.
Method.Field.Key.Method), but the last one in the chain is able to accept arguments.  
The particular use case in my application is for a func that returns
map[string]func(...interface{}) , so it can be invoked as Func.Key (optional args to the
func referenced by Key)
@rsc
Copy link
Contributor

@rsc rsc commented Feb 28, 2012

Comment 3:

I agree that your mapNiladicFunc example should probably work: that is
likely a bug.  The issue is that the parser is treating
{{.F.key}}
as
{{.F .key}}
It stopped parsing the chain once it had seen the F (F=mapNiladicFunc here).
I don't understand how your mapVariadicFunc example is supposed to work,
and that is what I primarily based my reply on.  Did you mean s/.key// in that
test case?
@robfig
Copy link
Contributor

@robfig robfig commented Feb 28, 2012

Comment 4:

I'm sorry, you're right -- I should have put it as a map value.  
+func mapVariadicFunc() map[string]func(...interface{})string {
+   return map[string]{
+       "key": func(args ...interface{})string {
+           return fmt.Sprintf("args=%s", args)
+       },
+   }
+}
@rsc
Copy link
Contributor

@rsc rsc commented Feb 28, 2012

Comment 5:

Okay, well then mapVariadicFunc.key 1 2 3 cannot work,
because you are grabbing the .key before passing the args.
But we should probably fix mapNiladicFunc.
@robfig
Copy link
Contributor

@robfig robfig commented Feb 28, 2012

Comment 6:

Sorry, I didn't understand what you meant.  
"mapVariadicFunc.key" resolves to a func(...interface{}) .. do you mean the args have
already been applied by the time it's resolved?  I didn't see any situation in the docs
where args would be applied to something other than the final value on the left side. 
(after all of the dots have been processed).  
It sounds like it's the case that the template doesn't differentiate between the dot and
non-dot tokens, and that's the fundamental reason why it doesn't behave the way I
expected?
I added a couple more tests to cover structs (which I still think apply, even if my
understanding was off on the mapVariadicFunc case).
Thanks!
diff -r c1702f36df03 src/pkg/template/exec_test.go
--- a/src/pkg/template/exec_test.go Tue Oct 18 10:55:12 2011 +1100
+++ b/src/pkg/template/exec_test.go Tue Feb 28 13:36:48 2012 -0500
@@ -402,6 +402,14 @@
    {"error method, error", "{{.EPERM true}}", "", tVal, false},
    {"error method, no error", "{{.EPERM false}}", "false", tVal, true},
 
+   // Func chaining.
+   {"Func.Key: String", "{{mapString.key}}", "value", tVal, false},
+   {"Func.Key: Func()", "{{mapNiladicFunc.key}}", "value", tVal, true},
+   {"Func.Key: Func(...)", "{{mapVariadicFunc.key 1 2 3}}", "args=[1, 2, 3]", tVal, true},
+   {"Func.Field.Field: String", "{{tVal.U.V}}", "v", tVal, true},
+   {"Func.Method.Field: String", "{{tVal.GetU.V}}", "v", tVal, true},
+   {"Func.Method(...)", "{{tVal.Method1 1}}", "1", tVal, true},
+
    // Fixed bugs.
    // Must separate dot and receiver; otherwise args are evaluated with dot set to variable.
    {"bug0", "{{range .MSIone}}{{if $.Method1 .}}X{{end}}{{end}}", "X", tVal, true},
@@ -431,6 +439,32 @@
    return "oneArg=" + a
 }
 
+func mapString() map[string]string {
+   return map[string]string{
+       "key": "value",
+   }
+}
+
+func mapNiladicFunc() map[string]func()string {
+   return map[string]func()string {
+       "key": func() string {
+           return "value"
+       },
+   }
+}
+
+func mapVariadicFunc() map[string]func(...interface{})string {
+   return map[string]func(...interface{})string {
+       "key": func(args ...interface{})string {
+           return fmt.Sprintf("args=%s", args)
+       },
+   }
+}
+
+func tValFunc() *T {
+   return tVal
+}
+
 // count returns a channel that will deliver n sequential 1-letter strings starting at "a"
 func count(n int) chan string {
    if n == 0 {
@@ -459,6 +493,10 @@
        "typeOf":   typeOf,
        "vfunc":    vfunc,
        "zeroArgs": zeroArgs,
+       "mapString": mapString,
+       "mapNiladicFunc": mapNiladicFunc,
+       "mapVariadicFunc": mapVariadicFunc,
+       "tVal": tValFunc,
    }
    for _, test := range execTests {
        tmpl := New(test.name).Funcs(funcs)
@rsc
Copy link
Contributor

@rsc rsc commented Feb 28, 2012

Comment 7:

Okay, I am going to stop making a fool of myself and let Rob handle this.
I don't have a working Go system right now (in the middle of some changes)
so I am running these in my head and not doing a very good job.
The last batch of tests you posted does look plausible.
@robpike
Copy link
Contributor

@robpike robpike commented Mar 3, 2012

Comment 8:

I spent some time on this and although there are easy fixes for some of the details I
would prefer not to make them right now, but rather wait until after Go 1 is released.
The chaining bug (mapString.key) is maybe just an oversight, and is easy to fix but not
as cleanly as I would like. I dabbled in a deep change to the evaluator with good
results, but there are subtle semantic changes that urge caution.
The case where there is no following field is tricky because it is ambiguous whether the
function should be called. Consider
  {{f}}
If is a function, we call it. But if f is a function returning a function, do we call it
or don't we? It's not obvious. We don't have the f() notation to tell us now many calls
to make: f f() f()() f()()() etc.
Even in apparently more clear-cut cases, there are ambiguities. Consider
  {{f 1}}
This could mean f takes one argument and we call f(1), or it could mean f is a niladic
function-valued function that we call once, then use that: f()(1). This ambiguity
bothers me; I would like the syntax to tell me what to do.
Templates do not have general evaluation capability, and this issue report touches one
or two areas where that can be problematic.  Since there are workarounds for these
examples no worse than a custom function for some cases, I would prefer to wait until
after Go 1, when I plan to do some major work on the evaluator.

Labels changed: added priority-later, removed priority-go1.

@rsc
Copy link
Contributor

@rsc rsc commented Mar 3, 2012

Comment 9:

Should we go back to never calling a function in a struct field?
@robpike
Copy link
Contributor

@robpike robpike commented Mar 3, 2012

Comment 10:

Perhaps. although it's a bit disruptive to back out.
@robpike
Copy link
Contributor

@robpike robpike commented Mar 3, 2012

Comment 11:

This issue was closed by revision f1d3ff1.

Status changed to Fixed.

@robpike
Copy link
Contributor

@robpike robpike commented Mar 11, 2012

Comment 12:

Issue #3283 has been merged into this issue.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
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
4 participants
You can’t perform that action at this time.