code review 5137041: go/doc, godoc, gotest: support for reading example docu...

go/doc, godoc, gotest: support for reading example documentation

This CL introduces the go.Example type and go.Examples functions that
are used to represent and extract code samples from Go source.

They should be of the form:

// Output of this function.
func ExampleFoo() {
        fmt.Println("Output of this function.")
}

It also modifies godoc to read example code from _test.go files,
and include them in the HTML output with JavaScript-driven toggles.

It also implements testing of example functions with gotest.
The stdout/stderr is compared against the output comment on the
function.

This CL includes examples for the sort.Ints function and the
sort.SortInts type. After patching this CL in and re-building go/doc
and godoc, try
        godoc -http=localhost:6060
and visit http://localhost:6060/pkg/sort/

[adg, 2011-09-25 08:13:44]

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

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

[gri, 2011-09-26 22:22:06]

This looks reasonable to me as well.

I think a toggle (per rsc's suggestion) makes sense. We should probably come up
with a bit of a standard approach for the various display modes supported by
godoc.

[adg, 2011-09-27 04:22:09]

I've updated the CL with modifications to gotest, also, and now show the
examples in the HTML view instead. Haven't done the collapsing toggle yet,
but that's easily done.

[+r for gotest changes]

On 27 September 2011 08:22, <gri@golang.org> wrote:

> This looks reasonable to me as well.
>
> I think a toggle (per rsc's suggestion) makes sense. We should probably
> come up with a bit of a standard approach for the various display modes
> supported by godoc.
>
>
>
http://codereview.appspot.com/**5137041/<http://codereview.appspot.com/5137041/>
>

[adg, 2011-09-28 00:02:52]

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

Please take another look.

[rsc, 2011-10-05 15:29:00]

Seems reasonable.  Examples should be a slice, not a map.
The order is important: they should run in the order they are
in the file.

Russ

[adg, 2011-10-05 23:07:15]

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

Please take another look.

[gri, 2011-10-05 23:16:40]

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go#newcod...
src/cmd/godoc/godoc.go:470: // unindent and remove surrounding braces
This is very specific to the current formatting. Wouldn't it be better to have a
node_htmlFunc that takes a statement list instead?

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go#newcod...
src/cmd/godoc/godoc.go:598: exampleHTML *template.Template
this list is ordered

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go#newcod...
src/cmd/godoc/godoc.go:612: exampleHTML = readTemplate("example.html")
same here - please reorder

http://codereview.appspot.com/5137041/diff/7001/src/pkg/go/doc/example.go
File src/pkg/go/doc/example.go (right):

http://codereview.appspot.com/5137041/diff/7001/src/pkg/go/doc/example.go#new...
src/pkg/go/doc/example.go:17: Name   string         // package identifier
s/identifier/name/

http://codereview.appspot.com/5137041/diff/7001/src/pkg/go/doc/example.go#new...
src/pkg/go/doc/example.go:47: // We don't want TesticularCancer.
how about "We don't want "Testing". instead...

http://codereview.appspot.com/5137041/diff/7001/src/pkg/testing/example.go
File src/pkg/testing/example.go (right):

http://codereview.appspot.com/5137041/diff/7001/src/pkg/testing/example.go#ne...
src/pkg/testing/example.go:25: if err, ok := e.(os.Error); ok {
can't you just do:

log.Fatal(e.(os.Error))

If it's not an os.Error, the type assertion will fail and re-panic.

[adg, 2011-10-05 23:29:55]

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go#newcod...
src/cmd/godoc/godoc.go:470: // unindent and remove surrounding braces
On 2011/10/05 23:16:41, gri wrote:
> This is very specific to the current formatting. Wouldn't it be better to have
a
> node_htmlFunc that takes a statement list instead?

Can I get your guidance as to how to write such a function? I tried printing a
statement list and had trouble.

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go#newcod...
src/cmd/godoc/godoc.go:598: exampleHTML *template.Template
On 2011/10/05 23:16:41, gri wrote:
> this list is ordered

Done.

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go#newcod...
src/cmd/godoc/godoc.go:612: exampleHTML = readTemplate("example.html")
On 2011/10/05 23:16:41, gri wrote:
> same here - please reorder

Done.

http://codereview.appspot.com/5137041/diff/7001/src/pkg/go/doc/example.go
File src/pkg/go/doc/example.go (right):

http://codereview.appspot.com/5137041/diff/7001/src/pkg/go/doc/example.go#new...
src/pkg/go/doc/example.go:17: Name   string         // package identifier
On 2011/10/05 23:16:41, gri wrote:
> s/identifier/name/

Changed it to the more accurate "name of the item being demonstrated"

http://codereview.appspot.com/5137041/diff/7001/src/pkg/go/doc/example.go#new...
src/pkg/go/doc/example.go:47: // We don't want TesticularCancer.
On 2011/10/05 23:16:41, gri wrote:
> how about "We don't want "Testing". instead... 

This was Rob's original comment from gotest. I have updated it.

http://codereview.appspot.com/5137041/diff/7001/src/pkg/testing/example.go
File src/pkg/testing/example.go (right):

http://codereview.appspot.com/5137041/diff/7001/src/pkg/testing/example.go#ne...
src/pkg/testing/example.go:25: if err, ok := e.(os.Error); ok {
On 2011/10/05 23:16:41, gri wrote:
> can't you just do:
> 
> log.Fatal(e.(os.Error))
> 
> If it's not an os.Error, the type assertion will fail and re-panic.

It'll re-panic with a type assertion failure, not the original panic. This will
re-panic with any value.

[r, 2011-10-05 23:30:59]

testing stuff LGTM but can maybe be refactored.

http://codereview.appspot.com/5137041/diff/11001/src/pkg/testing/example.go
File src/pkg/testing/example.go (right):

http://codereview.appspot.com/5137041/diff/11001/src/pkg/testing/example.go#n...
src/pkg/testing/example.go:22: func RunExamples(examples []InternalExample) (ok
bool) {
this is so much like the regular tester. can it be refactored? Fine to do that
in another CL.

[gri, 2011-10-06 00:03:05]

LGTM

rsc may want to comment on the testing changes.

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

http://codereview.appspot.com/5137041/diff/7001/src/cmd/godoc/godoc.go#newcod...
src/cmd/godoc/godoc.go:470: // unindent and remove surrounding braces
On 2011/10/05 23:29:56, adg wrote:
> On 2011/10/05 23:16:41, gri wrote:
> > This is very specific to the current formatting. Wouldn't it be better to
have
> a
> > node_htmlFunc that takes a statement list instead?
> 
> Can I get your guidance as to how to write such a function? I tried printing a
> statement list and had trouble.

After further consideration, this is probably ok for now, especially if the
output is tested. Doing it differently would require the go/printer to accept a
statement list, and I am reluctant to make that change for now.

http://codereview.appspot.com/5137041/diff/14002/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

http://codereview.appspot.com/5137041/diff/14002/src/cmd/godoc/godoc.go#newco...
src/cmd/godoc/godoc.go:995: return PageInfo{Dirname: abspath, Err: err}
This seems too harsh. If we hadn't looked at the test files, we might be ok and
could still serve documentation.

I would simple ignore the examples in this case but continue; and perhaps log
this situation. Generally godoc logs only if -v is set, need to verify if this
is also the case for erroneous situations like here (or if logging always
happens in this case).

[adg, 2011-10-06 00:39:14]

http://codereview.appspot.com/5137041/diff/14002/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

http://codereview.appspot.com/5137041/diff/14002/src/cmd/godoc/godoc.go#newco...
src/cmd/godoc/godoc.go:995: return PageInfo{Dirname: abspath, Err: err}
On 2011/10/06 00:03:05, gri wrote:
> This seems too harsh. If we hadn't looked at the test files, we might be ok
and
> could still serve documentation.
> 
> I would simple ignore the examples in this case but continue; and perhaps log
> this situation. Generally godoc logs only if -v is set, need to verify if this
> is also the case for erroneous situations like here (or if logging always
> happens in this case).

This will throw an error if *_test.go can't be parsed. IMO we should log such
errors regardless of *verbose.

[rsc, 2011-10-06 17:40:52]

LGTM

[adg, 2011-10-06 18:56:22]

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

go/doc, godoc, gotest: support for reading example documentation

This CL introduces the go.Example type and go.Examples functions that
are used to represent and extract code samples from Go source.

They should be of the form:

// Output of this function.
func ExampleFoo() {
        fmt.Println("Output of this function.")
}

It also modifies godoc to read example code from _test.go files,
and include them in the HTML output with JavaScript-driven toggles.

It also implements testing of example functions with gotest.
The stdout/stderr is compared against the output comment on the
function.

This CL includes examples for the sort.Ints function and the
sort.SortInts type. After patching this CL in and re-building go/doc
and godoc, try
        godoc -http=localhost:6060
and visit http://localhost:6060/pkg/sort/

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

[rog, 2011-10-07 08:40:28]

sorry for this late comment, but i've only just got
around to looking at this. looks really good, but a few
small remarks:

- ironically, this feature doesn't seem to be mentioned anywhere
in the documentation (although i've quite possibly missed something),
so if you haven't seen this CL go by, you might not know how to do examples.

- if there was such documentation, it should probably mention
that the name of the example is used to decide what type
or function to attach it to (perhaps i should have found that
obvious, but i didn't)

- for examples that use several features from a package,
it would be nice if there was some way of attaching examples
to the top of a godoc page rather than to a specific function
or type. Perhaps if the example name was not recognised,
the example could go at the top, preceded by the doc comments
for that example function. that would also make it obvious when
you have misspelled an example and so left it out accidentally.

- even though the source is shown directly, it might be nice
to link to the source code in context anyway.

On 6 October 2011 19:56,  <adg@golang.org> wrote:
> *** Submitted as
> http://code.google.com/p/go/source/detail?r=e23057584c65 ***
>
> go/doc, godoc, gotest: support for reading example documentation
>
> This CL introduces the go.Example type and go.Examples functions that
> are used to represent and extract code samples from Go source.
>
> They should be of the form:
>
> // Output of this function.
> func ExampleFoo() {
>        fmt.Println("Output of this function.")
> }
>
> It also modifies godoc to read example code from _test.go files,
> and include them in the HTML output with JavaScript-driven toggles.
>
> It also implements testing of example functions with gotest.
> The stdout/stderr is compared against the output comment on the
> function.
>
> This CL includes examples for the sort.Ints function and the
> sort.SortInts type. After patching this CL in and re-building go/doc
> and godoc, try
>        godoc -http=localhost:6060
> and visit http://localhost:6060/pkg/sort/
>
> R=gri, r, rsc
> CC=golang-dev
> http://codereview.appspot.com/5137041
>
>
> http://codereview.appspot.com/5137041/
>