code review 4667070: exp/template: variable evaluation.

exp/template: variable evaluation.
Still need to do static checking of declarations during parse.

[r, 2011-07-08 23:52:45]

Hello golang-dev@googlegroups.com,

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

[dsymonds, 2011-07-09 02:01:28]

LGTM

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

http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec.go#new...
src/pkg/exp/template/exec.go:55: func (s *state) value(name string)
reflect.Value {
maybe call this method "var" rather than "value"?

http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec.go#new...
src/pkg/exp/template/exec.go:207: // Set $x to the element rather than the map.
s/element/key/

(element of a map usually refers to the map value, such as in reflect)

http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec.go#new...
src/pkg/exp/template/exec.go:275: case *fieldNode:
why move this down?

http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec_test.go
File src/pkg/exp/template/exec_test.go (right):

http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec_test.g...
src/pkg/exp/template/exec_test.go:158: // Variables.
Can you add a test case for a nested var definition?

[r2, 2011-07-09 02:04:28]

On 09/07/2011, at 12:01 PM, dsymonds@golang.org wrote:

> LGTM
> 
> 
> http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec.go
> File src/pkg/exp/template/exec.go (right):
> 
>
http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec.go#new...
> src/pkg/exp/template/exec.go:55: func (s *state) value(name string)
> reflect.Value {
> maybe call this method "var" rather than "value"?

var is a keyword.

> 
>
http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec.go#new...
> src/pkg/exp/template/exec.go:207: // Set $x to the element rather than
> the map.
> s/element/key/
> 
> (element of a map usually refers to the map value, such as in reflect)
> 
>
http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec.go#new...
> src/pkg/exp/template/exec.go:275: case *fieldNode:
> why move this down?

alphabetical or
> http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec_test.go
> File src/pkg/exp/template/exec_test.go (right):
> 
>
http://codereview.appspot.com/4667070/diff/1/src/pkg/exp/template/exec_test.g...
> src/pkg/exp/template/exec_test.go:158: // Variables.
> Can you add a test case for a nested var definition?
> 
> http://codereview.appspot.com/4667070/

[r, 2011-07-09 02:05:51]

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

exp/template: variable evaluation.
Still need to do static checking of declarations during parse.

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