code review 4169042: template: reverse order of arguments to Execute

template: reverse order of arguments to Execute

In line with other functions such as Fprintf, put the
thing to be written first.

Apologies for the breakages this is sure to cause.

[r, 2011-02-09 20:32:46]

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

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

[eds, 2011-02-09 21:08:38]

On Thu, Feb 10, 2011 at 9:32 AM,  <r@golang.org> wrote:
> Reviewers: rsc, gri, adg,
>
> Message:
> Hello rsc, gri, adg (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> template: reverse order of arguments to Execute
>
> In line with other functions such as Fprintf, put the
> thing to be written first.
>
> Apologies for the breakages this is sure to cause.
>
> Please review this at http://codereview.appspot.com/4169042/
>
> Affected files:
>  M doc/codelab/wiki/final.go
>  M doc/codelab/wiki/index.html
>  M doc/effective_go.html
>  M misc/goplay/goplay.go
>  M src/cmd/godoc/godoc.go
>  M src/cmd/godoc/main.go
>  M src/cmd/goinstall/make.go
>  M src/pkg/rpc/debug.go
>  M src/pkg/template/template.go
>  M src/pkg/template/template_test.go
>  M test/chan/select5.go
>
>
>

src/pkg/exp/eval/gen.go also makes use of this method.

- Evan

[gri, 2011-02-09 21:10:29]

LGTM

But I am not sure about the HTML escape changes in the doc.

http://codereview.appspot.com/4169042/diff/1/doc/codelab/wiki/index.html
File doc/codelab/wiki/index.html (right):

http://codereview.appspot.com/4169042/diff/1/doc/codelab/wiki/index.html#newc...
doc/codelab/wiki/index.html:415: fmt.Fprintf(w, "<h1>Editing %s</h1>"+
if other documents (go_spec.html) we escape < and > also within <pre>'s.

[r2, 2011-02-09 21:15:01]

On Feb 9, 2011, at 1:10 PM, gri@golang.org wrote:

> LGTM
> 
> But I am not sure about the HTML escape changes in the doc.
> 
> 
> http://codereview.appspot.com/4169042/diff/1/doc/codelab/wiki/index.html
> File doc/codelab/wiki/index.html (right):
> 
>
http://codereview.appspot.com/4169042/diff/1/doc/codelab/wiki/index.html#newc...
> doc/codelab/wiki/index.html:415: fmt.Fprintf(w, "<h1>Editing %s</h1>"+
> if other documents (go_spec.html) we escape < and > also within <pre>'s.

hmm.  neither am i. plus i didn't make them by hand; they were done by some
process.

i believe the change is because GenHTML was removed from the printer.

adg?

-rob

[aam, 2011-02-09 21:39:24]

> i believe the change is because GenHTML was removed from the printer.

srcextract.go (in the same directory as the wiki codelab) used
printer.GenHTML to extract the source code samples.

i can submit a CL with a fix, or you can add the following patch to
the current CL. or we can wait for adg :)


$ hg diff
diff -r c03d0223f209 doc/codelab/wiki/srcextract.go
--- a/doc/codelab/wiki/srcextract.go	Wed Feb 09 13:13:17 2011 -0800
+++ b/doc/codelab/wiki/srcextract.go	Wed Feb 09 14:37:47 2011 -0700
@@ -9,6 +9,7 @@
 	"go/token"
 	"log"
 	"os"
+	"template"
 )

 var (
@@ -67,5 +68,10 @@
 		}
 	}
 	// output
+	if *html {
+		nb := new(bytes.Buffer)
+		template.HTMLEscape(nb, b.Bytes())
+		b = nb
+	}
 	b.WriteTo(os.Stdout)
 }

[r, 2011-02-09 22:23:03]

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

template: reverse order of arguments to Execute

In line with other functions such as Fprintf, put the
thing to be written first.

Apologies for the breakages this is sure to cause.

R=rsc, gri, adg, eds, r2, aam
CC=golang-dev
http://codereview.appspot.com/4169042