code review 4808089: exp/template/html: New package with a toy template tran...

exp/template/html: New package with a toy template transformation.

func Reverse(*Template) *Template
returns a template that produces the reverse of the original
for any input.

Changes outside exp/template/html include:
- Adding a getter for a template's FuncMap so that derived templates
  can inherit function definitions.
- Exported one node factory function, newIdentifier.
  Deriving tempaltes requires constructing new nodes, but I didn't
  export all of them because I think shallow copy functions might
  be more useful for this kind of work.
- Bugfix: Template's Name() method ignores the name field so
  template.New("foo") is a nil dereference instead of "foo".

Caveats: Reverse is a toy.  It is not UTF-8 safe, and does not
preserve order of calls to funcs in FuncMap.

For context, see http://groups.google.com/group/golang-nuts/browse_thread/thread/e8bc7c771aae3f20/b1ac41dc6f609b6e?lnk=gst

[MikeSamuel, 2011-08-10 19:24:05]

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

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

[rsc, 2011-08-10 21:01:18]

Do you avoid the accessors if you just edit the template in place?

That's what I was expecting: it's more efficient, and we 
made sure that each template is only referred to as
part of one *Set precisely so that editing them in place
would be valid.

http://codereview.appspot.com/4808089/diff/8008/src/pkg/exp/template/html/rev...
File src/pkg/exp/template/html/reverse_template.go (right):

http://codereview.appspot.com/4808089/diff/8008/src/pkg/exp/template/html/rev...
src/pkg/exp/template/html/reverse_template.go:1: // Copyright 2011 The Go
Authors. All rights reserved.
file name is kind of long.  reverse.go would be fine.

[MikeSamuel, 2011-08-10 23:13:52]

On 2011/08/10 21:01:18, rsc wrote:
> Do you avoid the accessors if you just edit the template in place?
> 
> That's what I was expecting: it's more efficient, and we 
> made sure that each template is only referred to as
> part of one *Set precisely so that editing them in place
> would be valid.

I implemented that.  Please see the difference between snapshot 4 and 5.

I was worried that exp/template/parse.go might (or might in the future) share
common sub-expressions.  I have run into parse "trees" that aren't really trees
for that reason, but I'm perfectly happy to do in-place transformations if
that's not unduly limiting the development of exp/template.

With this change, the FuncMap method in exp/template/parse.go is no longer used.
 Let me know whether that should stay or go.

[MikeSamuel, 2011-08-10 23:13:58]

http://codereview.appspot.com/4808089/diff/8008/src/pkg/exp/template/html/rev...
File src/pkg/exp/template/html/reverse_template.go (right):

http://codereview.appspot.com/4808089/diff/8008/src/pkg/exp/template/html/rev...
src/pkg/exp/template/html/reverse_template.go:1: // Copyright 2011 The Go
Authors. All rights reserved.
On 2011/08/10 21:01:18, rsc wrote:
> file name is kind of long.  reverse.go would be fine.

Done.  Please see the diffs between snapshots 3 and 4.

[rsc, 2011-08-11 01:31:58]

> I was worried that exp/template/parse.go might (or might in the future)
> share common sub-expressions.  I have run into parse "trees" that aren't
> really trees for that reason, but I'm perfectly happy to do in-place
> transformations if that's not unduly limiting the development of
> exp/template.

we planned for modification from the start.

[r, 2011-08-11 03:57:17]

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:7: 
the package comment should be bound to the package clause with no intervening
blank line, so delete the blank line 7.
this is just a placeholder, but still, something like:

// Package html is a specialization of the exp/template package that automates
// construction of safe HTML output.
// At the moment it is just skeleton code that....

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:16: // Reverses a template in-place so that
after Reverse(t), t.Execute(wr, data)
doc comments should be a sentence starting with the name of the item. see the
section on comments in Effective Go.

Reverse reverses ....

and similarly below

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:23: t.Funcs(supportFuncs)
this will crash if i call twice - can't add two functions with the same name

since this is only a toy i don't think it matters, though

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:26: // joining then we will need to avoid
multiply reversing the same tree.
it won't, but a fine TODO to keep around

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:41: // TODO: Handle other inner node types.
default:
 panic("unknown node: " + node.Type().String())
or something like that.

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:62: // The reverse of a text node has the
text reversed by bit.
s/bit/byte/

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:105: copy(augmentedCommands, cmds)
augmentedCommands is quite a mouthful...

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:124: supportFuncs["reverse"] = func(x
interface{}) string {
why is this a closure? why not a proper function, so you don't need the init
func at all

var supportFuncs = template.FuncMap{
  "reverse": reverse,
}

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:131: s = string(x.([]byte))
these are unnecessary type assertions. do
switch y := x.(type) 
and then y will have the appropriate type in each case.
e.g. here it would be just s = string(y)

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
File src/pkg/exp/template/html/reverse_test.go (right):

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse_test.go:29: t.Errorf("Failed to parse
template: %s", err)
no initial caps in error strings. they're not sentences, really

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse_test.go:46: t.Errorf("Reversed output: %s !=
%s", golden, actual)
s/%s/%q/g

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

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/parse.go#n...
src/pkg/exp/template/parse.go:27: return t.name
oops

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/parse.go#n...
src/pkg/exp/template/parse.go:52: func (t *Template) FuncMap() FuncMap {
do you use this?

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/parse/node.go
File src/pkg/exp/template/parse/node.go (right):

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/parse/node...
src/pkg/exp/template/parse/node.go:157: func NewIdentifier(ident string)
*IdentifierNode {
// NewIdentifier returns a new IdentifierNode with the given identifier name.

[MikeSamuel, 2011-08-11 06:17:04]

Incremental diffs at snapshot 7 : 
http://codereview.appspot.com/4808089/diff2/11:21001/src/pkg/exp/template/htm...

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:7: 
On 2011/08/11 03:57:17, r wrote:
> the package comment should be bound to the package clause with no intervening
> blank line, so delete the blank line 7.
> this is just a placeholder, but still, something like:
> 
> // Package html is a specialization of the exp/template package that automates
> // construction of safe HTML output.
> // At the moment it is just skeleton code that....

Done.  When is it appropriate to use a doc.go for docs instead of top-of-file
comments?

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:16: // Reverses a template in-place so that
after Reverse(t), t.Execute(wr, data)
On 2011/08/11 03:57:17, r wrote:
> doc comments should be a sentence starting with the name of the item. see the
> section on comments in Effective Go.
> 
> Reverse reverses ....
> 
> and similarly below

Done for all function definitions and var declarations in this file.  The
FuncMap method in parse.go already complies.

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:41: // TODO: Handle other inner node types.
On 2011/08/11 03:57:17, r wrote:
> default:
>  panic("unknown node: " + node.Type().String())
> or something like that.

Done.

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:62: // The reverse of a text node has the
text reversed by bit.
On 2011/08/11 03:57:17, r wrote:
> s/bit/byte/

As part of general comment cleanup this now-reads

// reverseTextNode reverses the text byte-by-byte.
// If the text is meant to be UTF-8 and contains non-7-bit sequences
// it will now be invalid.

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:105: copy(augmentedCommands, cmds)
On 2011/08/11 03:57:17, r wrote:
> augmentedCommands is quite a mouthful...

augmentedCommands -> newCmds

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:124: supportFuncs["reverse"] = func(x
interface{}) string {
On 2011/08/11 03:57:17, r wrote:
> why is this a closure? why not a proper function, so you don't need the init
> func at all
> 
> var supportFuncs = template.FuncMap{
>   "reverse": reverse,
> }

Done and done.

Somehow I concluded from the discussion of the unary & operator that the only
way to get a function pointer was by starting with a function literal, but I
found "A function declaration binds an identifier to a function (§Function
types)." on rereading.

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse.go:131: s = string(x.([]byte))
On 2011/08/11 03:57:17, r wrote:
> these are unnecessary type assertions. do
> switch y := x.(type) 
> and then y will have the appropriate type in each case.
> e.g. here it would be just s = string(y)

Lovely.  Fixed up the switch in reverseNode above too.

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
File src/pkg/exp/template/html/reverse_test.go (right):

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse_test.go:29: t.Errorf("Failed to parse
template: %s", err)
On 2011/08/11 03:57:17, r wrote:
> no initial caps in error strings. they're not sentences, really

Done in 3 places.

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/html/rever...
src/pkg/exp/template/html/reverse_test.go:46: t.Errorf("Reversed output: %s !=
%s", golden, actual)
On 2011/08/11 03:57:17, r wrote:
> s/%s/%q/g

Done.  Is the difference between %s and %q analagous to the difference between
Stringer and GoStringer?

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

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/parse.go#n...
src/pkg/exp/template/parse.go:52: func (t *Template) FuncMap() FuncMap {
On 2011/08/11 03:57:17, r wrote:
> do you use this?

I did in snapshot 1 but no longer.  Removed.

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/parse/node.go
File src/pkg/exp/template/parse/node.go (right):

http://codereview.appspot.com/4808089/diff/11/src/pkg/exp/template/parse/node...
src/pkg/exp/template/parse/node.go:157: func NewIdentifier(ident string)
*IdentifierNode {
On 2011/08/11 03:57:17, r wrote:
> // NewIdentifier returns a new IdentifierNode with the given identifier name.

Done.

[r, 2011-08-11 06:36:55]

%q prints a quoted string, or quotes the strings inside an object
containing strings such as a slice of strings.

-rob

[r, 2011-08-11 06:48:50]

this is pretty good. just a few idiomatic improvements.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:45: panic("handling for type " +
node.String() + "not implemented.")
node.String() will print the node, so s/ type//

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:50: // reverseListNode recursively recurses
its input's children and reverses their
"recursively recurses" is redundantly redundant.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:70: // it will now be invalid.
it's only a toy but it might be good practice. it's not very hard to fix this
issue, but i have no problems with leaving it as is.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:93: if len(lastCommand.Args) != 0 {
it would be idiomatic Go to write

if lastCommand := cmds[nCmds-1]; len(lastCommand.Args) > 0 {

this avoids an indentation and also makes the scoping crystal clear

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:95: case *parse.IdentifierNode:
nothing wrong with this except mild verbosity. for a single entry you can type
assert:

if arg, ok := lastCommand.Args[0].(*parse.IdentifierNode); ok {
...
}

you could even fold further:

ok && arg.Ident == "reverse"

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:113: node.Pipe.Cmds = newCmds
this whole burst is just

node.Pipe.Cmds = append(node.Pipe.Cmds, &reverseCommand)

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:137: s = "<inconvertible>"
you could do
s = fmt.Sprintf("<inconvertible of type %T>", x)

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse_test.go (right):

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse_test.go:35: buffer :=
bytes.NewBuffer(make([]byte, 0, 1024))
no need for giving a slice.  yes, you can save an allocation but it's not worth
it in most cases.  NewBuffer is mostly to prep a buffer for reading and the
usual writing idiom is just
  buffer := new(bytes.Buffer)

[nigeltao, 2011-08-11 08:55:14]

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:141: var i int
Delete this line and use "i := 0" on the next one.

[MikeSamuel, 2011-08-11 19:08:01]

Incremental diffs at
http://codereview.appspot.com/4808089/diff2/21001:24003/src/pkg/exp/template/...

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:45: panic("handling for type " +
node.String() + "not implemented.")
On 2011/08/11 06:48:56, r wrote:
> node.String() will print the node, so s/ type//

Done.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:50: // reverseListNode recursively recurses
its input's children and reverses their
On 2011/08/11 06:48:56, r wrote:
> "recursively recurses" is redundantly redundant.

:) "recursively recurses" -> "recursively reverses"

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:70: // it will now be invalid.
On 2011/08/11 06:48:56, r wrote:
> it's only a toy but it might be good practice. it's not very hard to fix this
> issue, but i have no problems with leaving it as is.

Done.
There are a few special characters outside ASCII that I'll need to deal with in
escaping functions.

Rather than use (range string(node.Text)), I poked around for utf-8 handling
library functions so that any escaping functions I write work with Writers and
[]byte without rebuffering.

This does a buffer copy, because the first rune can contain more bytes than the
last which means they can't be swapped without shifting bytes in the middle
around which sounds O(n*2) to me.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:93: if len(lastCommand.Args) != 0 {
On 2011/08/11 06:48:56, r wrote:
> it would be idiomatic Go to write
> 
> if lastCommand := cmds[nCmds-1]; len(lastCommand.Args) > 0 {
> 
> this avoids an indentation and also makes the scoping crystal clear

Done, but I don't see how this reduces indentation.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:95: case *parse.IdentifierNode:
On 2011/08/11 06:48:56, r wrote:
> nothing wrong with this except mild verbosity. for a single entry you can type
> assert:
> 
> if arg, ok := lastCommand.Args[0].(*parse.IdentifierNode); ok {
> ...
> }
> 
> you could even fold further:
> 
> ok && arg.Ident == "reverse"

Done.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:113: node.Pipe.Cmds = newCmds
On 2011/08/11 06:48:56, r wrote:
> this whole burst is just
> 
> node.Pipe.Cmds = append(node.Pipe.Cmds, &reverseCommand)

Done.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:137: s = "<inconvertible>"
On 2011/08/11 06:48:56, r wrote:
> you could do
> s = fmt.Sprintf("<inconvertible of type %T>", x)

Done.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:141: var i int
On 2011/08/11 08:55:15, nigeltao wrote:
> Delete this line and use "i := 0" on the next one.

Done.

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse_test.go (right):

http://codereview.appspot.com/4808089/diff/21001/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse_test.go:35: buffer :=
bytes.NewBuffer(make([]byte, 0, 1024))
On 2011/08/11 06:48:56, r wrote:
> no need for giving a slice.  yes, you can save an allocation but it's not
worth
> it in most cases.  NewBuffer is mostly to prep a buffer for reading and the
> usual writing idiom is just
>   buffer := new(bytes.Buffer)

Done.

[r, 2011-08-12 01:31:51]

very close

http://codereview.appspot.com/4808089/diff/24003/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/24003/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:73: reversedText := make([]byte, n)
the simplest way is
runes := make([]int, string(node.Text))
reverse runes
node.Text = []byte(string(runes))

lots of allocations but it's very short.

another way is to create a bytes.Buffer and use DecodeLastRune to write them
reversed.

[MikeSamuel, 2011-08-12 03:42:49]

http://codereview.appspot.com/4808089/diff/24003/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/24003/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:73: reversedText := make([]byte, n)
On 2011/08/12 01:31:52, r wrote:
> the simplest way is
> runes := make([]int, string(node.Text))
> reverse runes
> node.Text = []byte(string(runes))
> 
> lots of allocations but it's very short.
> 
> another way is to create a bytes.Buffer and use DecodeLastRune to write them
> reversed.

I can't find any reference to a form of make that takes a string and it doesn't
build for me but I think I got close.

Incremental diff at
http://codereview.appspot.com/4808089/diff2/24003:29003/src/pkg/exp/template/...

[r, 2011-08-12 03:53:42]

http://codereview.appspot.com/4808089/diff/29003/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/29003/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:73: runes := make([]int, n)
my fault, i misled you. i was saying 
runes := []int(string(runes))
but put in a spurious make invocation.
then you can do a trivial reversal algorithm.
if you prefer to leave it as is, that's fine too

[MikeSamuel, 2011-08-12 04:18:25]

If this looks good, should I rework it in the next CL to do non-contextual
auto-escaping?  Autoescape(*Tempalte) appends "| HTMLEscape" to all actions
regardless of context?

http://codereview.appspot.com/4808089/diff/37002/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/37002/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:69: runes[i], runes[j] = runes[j], runes[i]
I reworked the two reversal loops (list children and text runes) to use double
assignment.

[r, 2011-08-12 04:19:33]

LGTM

http://codereview.appspot.com/4808089/diff/37002/src/pkg/exp/template/html/re...
File src/pkg/exp/template/html/reverse.go (right):

http://codereview.appspot.com/4808089/diff/37002/src/pkg/exp/template/html/re...
src/pkg/exp/template/html/reverse.go:71: node.Text = []byte(string(runes))
there we go

[r2, 2011-08-12 04:31:01]

On Aug 12, 2011, at 2:18 PM, mikesamuel@gmail.com wrote:

> If this looks good, should I rework it in the next CL to do
> non-contextual auto-escaping?  Autoescape(*Tempalte) appends "|
> HTMLEscape" to all actions regardless of context?

That's simple place to start. You already have lots of the pieces.

-rob

[r, 2011-08-12 04:34:37]

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

exp/template/html: New package with a toy template transformation.

func Reverse(*Template) *Template
returns a template that produces the reverse of the original
for any input.

Changes outside exp/template/html include:
- Adding a getter for a template's FuncMap so that derived templates
  can inherit function definitions.
- Exported one node factory function, newIdentifier.
  Deriving tempaltes requires constructing new nodes, but I didn't
  export all of them because I think shallow copy functions might
  be more useful for this kind of work.
- Bugfix: Template's Name() method ignores the name field so
  template.New("foo") is a nil dereference instead of "foo".

Caveats: Reverse is a toy.  It is not UTF-8 safe, and does not
preserve order of calls to funcs in FuncMap.

For context, see
http://groups.google.com/group/golang-nuts/browse_thread/thread/e8bc7c771aae3...

R=rsc, r, nigeltao, r
CC=golang-dev
http://codereview.appspot.com/4808089

Committer: Rob Pike <r@golang.org>