code review 4826052: exp/template: simplify the helper functions

exp/template: simplify the helper functions
- create a pair of "Must" functions, one for templates, one for sets
- regularize the return values of the parsers so they can be wrapped by Must
- delete all the old Must functions and methods.

[r, 2011-07-28 18:16:15]

Hello golang-dev@googlegroups.com,

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

[gri, 2011-07-28 18:58:08]

LGTM

Some questions/suggestions.

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

http://codereview.appspot.com/4826052/diff/1/src/pkg/exp/template/helper.go#n...
src/pkg/exp/template/helper.go:39: //checked against those in the set.
// checked

http://codereview.appspot.com/4826052/diff/1/src/pkg/exp/template/helper.go#n...
src/pkg/exp/template/helper.go:61: return nil, err
always return t ?

http://codereview.appspot.com/4826052/diff/1/src/pkg/exp/template/helper.go#n...
src/pkg/exp/template/helper.go:86: return nil, err
would it make sense to always return s, even in the presence of errors?

seems like a simpler "invariant"

http://codereview.appspot.com/4826052/diff/1/src/pkg/exp/template/helper.go#n...
src/pkg/exp/template/helper.go:133: return nil, err
always return set? (not so sure in this case)

http://codereview.appspot.com/4826052/diff/1/src/pkg/exp/template/helper.go#n...
src/pkg/exp/template/helper.go:173: return nil, err
always return s ? (also below)

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

http://codereview.appspot.com/4826052/diff/1/src/pkg/exp/template/set_test.go...
src/pkg/exp/template/set_test.go:107: _, err := set.Parse(setText1)
set, err := new(Set).Parse(setText1)

and get rid of the set := new(Set)

but keeping it is fine, too, for symmetry with the other lines

http://codereview.appspot.com/4826052/diff/1/src/pkg/exp/template/set_test.go...
src/pkg/exp/template/set_test.go:133: _, err := ParseSetFile("DOES NOT EXIST")
same here, and in all tests below

[r, 2011-07-28 20:34:55]

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

Please take another look.

[r2, 2011-07-28 20:34:59]

i like your suggestion

[gri, 2011-07-28 21:49:22]

LGTM

[r, 2011-07-28 23:19:21]

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

exp/template: simplify the helper functions
- create a pair of "Must" functions, one for templates, one for sets
- regularize the return values of the parsers so they can be wrapped by Must
- delete all the old Must functions and methods.

R=golang-dev, gri, r
CC=golang-dev
http://codereview.appspot.com/4826052