code review 4671056: exp/template: better template invocation

exp/template: better template invocation
1) Make the value optional ({{template "foo"}})
2) Allow the template identifier to be a thing of type *Template.
The second makes it easy to drop templates in to a set dynamically
during invocation.

[r, 2011-07-09 08:27:44]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[dsymonds, 2011-07-09 13:13:57]

http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec.go
File src/pkg/exp/template/exec.go (right):

http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec.go#...
src/pkg/exp/template/exec.go:453: // validate guarantees that the value is valid
and assignable to the type.
did you mean to rename this method?

http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec.go#...
src/pkg/exp/template/exec.go:456: s.errorf("invalid value; expected %T", typ)
You want %v; %T will print out "*reflect.commonType".

http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec_tes...
File src/pkg/exp/template/exec_test.go (right):

http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec_tes...
src/pkg/exp/template/exec_test.go:46: Tmpl *Template
It would be good to test whether data gets passed through to the subtemplate.

http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/parse.go
File src/pkg/exp/template/parse.go (right):

http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/parse.go...
src/pkg/exp/template/parse.go:473: return fmt.Sprintf("{{template %s}}", t.name)
is the name guaranteed to be nice? or should this be %q (and on line 475?).

[r2, 2011-07-09 13:23:31]

On 09/07/2011, at 11:13 PM, dsymonds@golang.org wrote:

> 
> http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec.go
> File src/pkg/exp/template/exec.go (right):
> 
>
http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec.go#...
> src/pkg/exp/template/exec.go:453: // validate guarantees that the value
> is valid and assignable to the type.
> did you mean to rename this method?

a glitch from an aborted rewrite. fixed.

>
http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec.go#...
> src/pkg/exp/template/exec.go:456: s.errorf("invalid value; expected %T",
> typ)
> You want %v; %T will print out "*reflect.commonType".

fixed.

>
http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec_tes...
> File src/pkg/exp/template/exec_test.go (right):
> 
>
http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/exec_tes...
> src/pkg/exp/template/exec_test.go:46: Tmpl *Template
> It would be good to test whether data gets passed through to the
> subtemplate.

it's hard because i'm going to put in static checking and it won't be possible
to write a test that gets through the parser. it's so simple i'm not worried
about it. the tests for static checking should cover it.

> http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/parse.go
> File src/pkg/exp/template/parse.go (right):
> 
>
http://codereview.appspot.com/4671056/diff/3001/src/pkg/exp/template/parse.go...
> src/pkg/exp/template/parse.go:473: return fmt.Sprintf("{{template %s}}",
> t.name)
> is the name guaranteed to be nice? or should this be %q (and on line
> 475?).

it's guaranteed readable and anyway a) this is only used during debugging and b)
it's a node, not a name, so quotes are out of place.

[r, 2011-07-09 13:23:39]

Hello golang-dev@googlegroups.com, dsymonds@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[dsymonds, 2011-07-09 13:54:33]

LGTM

[r, 2011-07-09 21:32:09]

*** Submitted as http://code.google.com/p/go/source/detail?r=d44b6e3d0e88 ***

exp/template: better template invocation
1) Make the value optional ({{template "foo"}})
2) Allow the template identifier to be a thing of type *Template.
The second makes it easy to drop templates in to a set dynamically
during invocation.

R=golang-dev, dsymonds
CC=golang-dev
http://codereview.appspot.com/4671056