code review 6523045: godoc: make examples editable and runnable in playground

godoc: make examples editable and runnable in playground

[adg, 2012-10-03 05:56:09]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[adg, 2012-10-03 06:00:59]

I'm still working on the UI for this. Please review the godoc changes primarily.

http://codereview.appspot.com/6523045/diff/14001/doc/play/playground.js
File doc/play/playground.js (right):

http://codereview.appspot.com/6523045/diff/14001/doc/play/playground.js#newcode1
doc/play/playground.js:1: // Copyright 2012 The Go Authors. All rights reserved.
This merely syncs playground.js from https://code.google.com/p/go-playground

[gri, 2012-10-03 17:49:21]

FYI

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/godoc.go#newc...
src/cmd/godoc/godoc.go:359: buf := new(bytes.Buffer)
I usually wrote

var buf bytes.Buffer

for this kind of buffer use in this file

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/main.go
File src/cmd/godoc/main.go (right):

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/main.go#newco...
src/cmd/godoc/main.go:357: } else if bp, err := build.Import(path, "",
build.FindOnly); err == nil && bp.Dir != "" && bp.ImportPath != "" {
didn't we discuss this earlier and decided it's not needed?

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/main.go#newco...
src/cmd/godoc/main.go:485: defer resp.Body.Close()
can't this simply be done after the Copy, w/o defer?

[adg, 2012-10-03 21:50:03]

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/godoc.go#newc...
src/cmd/godoc/godoc.go:359: buf := new(bytes.Buffer)
On 2012/10/03 17:49:21, gri wrote:
> I usually wrote
> 
> var buf bytes.Buffer
> 
> for this kind of buffer use in this file

Done.

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/main.go
File src/cmd/godoc/main.go (right):

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/main.go#newco...
src/cmd/godoc/main.go:357: } else if bp, err := build.Import(path, "",
build.FindOnly); err == nil && bp.Dir != "" && bp.ImportPath != "" {
On 2012/10/03 17:49:21, gri wrote:
> didn't we discuss this earlier and decided it's not needed?

Yes, sorry. I forgot to revert the change after deleting the old CL.

https://codereview.appspot.com/6523045/diff/14001/src/cmd/godoc/main.go#newco...
src/cmd/godoc/main.go:485: defer resp.Body.Close()
On 2012/10/03 17:49:21, gri wrote:
> can't this simply be done after the Copy, w/o defer?

Done.

[adg, 2012-10-03 21:50:30]

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

Please take another look.

[gri, 2012-10-03 21:53:37]

LGTM

(.go changes)

[adg, 2012-10-04 05:46:50]

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

Please take another look.

[adg, 2012-10-04 05:51:52]

Hi Dave, can you please review the javascript/template parts of this? gri has
already reviewed the go code.

http://codereview.appspot.com/6523045/diff/20002/doc/play/playground.js
File doc/play/playground.js (right):

http://codereview.appspot.com/6523045/diff/20002/doc/play/playground.js#newcode1
doc/play/playground.js:1: // Copyright 2012 The Go Authors. All rights reserved.
This is just synchronizing playground.js with the go-playground repo.

[dsymonds, 2012-10-04 06:00:13]

LGTM

only looked at the non .go files.

https://codereview.appspot.com/6523045/diff/10007/doc/play/playground.js
File doc/play/playground.js (right):

https://codereview.appspot.com/6523045/diff/10007/doc/play/playground.js#newc...
doc/play/playground.js:122: if (window.history &&
unwrap? this is hard to read.

https://codereview.appspot.com/6523045/diff/10007/lib/godoc/example.html
File lib/godoc/example.html (right):

https://codereview.appspot.com/6523045/diff/10007/lib/godoc/example.html#newc...
lib/godoc/example.html:15: <a class="fmt" title="Fromat this code">Format</a>
typo: "Format"

[adg, 2012-10-04 06:53:11]

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

godoc: make examples editable and runnable in playground

R=dsymonds
CC=golang-dev
http://codereview.appspot.com/6523045

http://codereview.appspot.com/6523045/diff/10007/doc/play/playground.js
File doc/play/playground.js (right):

http://codereview.appspot.com/6523045/diff/10007/doc/play/playground.js#newco...
doc/play/playground.js:122: if (window.history &&
On 2012/10/04 06:00:13, dsymonds wrote:
> unwrap? this is hard to read.

This is syncing to the playground.js in the go-playground repo, so I won't make
the change in this CL, but I agree.

http://codereview.appspot.com/6523045/diff/10007/lib/godoc/example.html
File lib/godoc/example.html (right):

http://codereview.appspot.com/6523045/diff/10007/lib/godoc/example.html#newco...
lib/godoc/example.html:15: <a class="fmt" title="Fromat this code">Format</a>
On 2012/10/04 06:00:13, dsymonds wrote:
> typo: "Format"

Done.