Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
LGTM i have some qualms about merging if and for but they can wait. http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go File src/pkg/exp/template/parse.go (right): http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:85: return &listNode{nodeType: nodeText} nodeList? http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:218: n.int64 = i you could drop the special case here if you wanted. if i == 0 ... http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:220: // If an integer extraction succeeded, promote the float. 220,227d 242d ? http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:442: // Range keyword is past. 444 disagrees http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:462: // End keyword is past. 464 disagrees http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:470: // Else keyword is past. 470 disagrees http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:497: t.errorf("first argument of command cannot be %q", token.val) s/first argument of // ? http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:506: t.errorf("first argument of command cannot be %q", token.val) s/first argument of // ? http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse_te... File src/pkg/exp/template/parse_test.go (right): http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse_te... src/pkg/exp/template/parse_test.go:14: type numberTest struct { type numberTest struct { text string n numberNode } http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse_te... src/pkg/exp/template/parse_test.go:64: if n.imaginary != test.imaginary { if !reflect.DeepEqual(n, test.n) { t.Errorf("newNumber(%q) = %+v, want %+v", test.text, n, test.n) } ? http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse_te... src/pkg/exp/template/parse_test.go:100: func num(s string) *numberNode { unused?
http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go File src/pkg/exp/template/parse.go (right): http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:85: return &listNode{nodeType: nodeText} On 2011/06/22 23:12:20, rsc wrote: > nodeList? all the others are xxxNode, and this is a "list node", so stet. http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:218: n.int64 = i On 2011/06/22 23:12:20, rsc wrote: > you could drop the special case here if you wanted. > if i == 0 ... need to consider that 0x23 is also a float. http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:220: // If an integer extraction succeeded, promote the float. On 2011/06/22 23:12:20, rsc wrote: > 220,227d > 242d > ? partially done. i deleted the comparisons since the compiler isnt' that fastidious anyway http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:442: // Range keyword is past. On 2011/06/22 23:12:20, rsc wrote: > 444 disagrees no it doesn't. it's expecting a meta. "range" is a message. http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:462: // End keyword is past. On 2011/06/22 23:12:20, rsc wrote: > 464 disagrees no it doesn't. it's expecting a meta. "end" is a message. http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:470: // Else keyword is past. On 2011/06/22 23:12:20, rsc wrote: > 470 disagrees no it doesn't. it's expecting a meta. http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:497: t.errorf("first argument of command cannot be %q", token.val) On 2011/06/22 23:12:20, rsc wrote: > s/first argument of // > ? Done. http://codereview.appspot.com/4629053/diff/6006/src/pkg/exp/template/parse.go... src/pkg/exp/template/parse.go:506: t.errorf("first argument of command cannot be %q", token.val) On 2011/06/22 23:12:20, rsc wrote: > s/first argument of // > ? Done.
*** Submitted as http://code.google.com/p/go/source/detail?r=6be80da495a0 *** First cut at the parser for the new template package. This is not a full grammar, but the pieces are there to implement whatever we converge on. R=rsc CC=golang-dev http://codereview.appspot.com/4629053
