code review 5502059: go/doc, godoc: show methods of anonymous fields

go/doc, godoc: show methods of anonymous fields

Missing: Handling of embedded interfaces.

Also, for reasons outlined in the previous CL (5500055), embedded
types have to be exported for its "inherited" methods to be visible.
This will be addressed w/ a subsequent CL.

[gri, 2011-12-21 19:18:45]

Hello golang-dev@googlegroups.com,

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

[rsc, 2011-12-21 19:32:20]

Can we drop the embedded and change the receiver?
It seems like that should be straightforward now that
the info is there.

The goal for this is to be able to make the current
godoc testing look like the old (golang.org/pkg/testing) one.
Whether a method is provided by an embedded
unexported field or directly is an implementation detail.

If the embedded field F inside struct T is a pointer,
then the receiver should be (T).
If the embedded field inside struct T is a value,
then the receiver should be (T) if the embedded
receiver is F, or (*T) if *F.

Russ

[gri, 2011-12-22 01:36:18]

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

Please take another look.

[r2, 2011-12-22 01:52:15]

the presentation looks much better. LGTM from that perspective.

-rob

[rsc, 2011-12-22 20:30:08]

LGTM

It looks like this is not affecting presentation at all anymore,
which seems like a good idea until all the necessary pieces are ready.

http://codereview.appspot.com/5502059/diff/1010/src/pkg/go/doc/doc.go
File src/pkg/go/doc/doc.go (right):

http://codereview.appspot.com/5502059/diff/1010/src/pkg/go/doc/doc.go#newcode76
src/pkg/go/doc/doc.go:76: if name == "" || name == "_" {
Is this a type name?  I don't think you can encounter _ as a type name
so I am a little confused.

http://codereview.appspot.com/5502059/diff/1010/src/pkg/go/doc/doc.go#newcode403
src/pkg/go/doc/doc.go:403: // TODO(gri) Should sort methods also by ptr vs
non-ptr receiver.
I suspect that would be a mistake, because it would be
alphabetical in so many cases that people would assume
it was alphabetical instead of being two runs of alphabetical
concatenated.  That is, if it looks alphabetical most of the time,
it should be alphabetical all of the time.

There are file systems with large block sizes that keep their
directories sorted within a block but not across blocks, and
I've seen bugs where people assume that the directory entries
being read off disk are sorted, because in small directories
(which fit in a single disk block) they are.

[gri, 2011-12-22 21:06:48]

Addressed.

FYI: I am moving export filtering into go/doc - this should then fix the fact
that unexported anonymous field's methods are missed at the moment.

http://codereview.appspot.com/5502059/diff/1010/src/pkg/go/doc/doc.go
File src/pkg/go/doc/doc.go (right):

http://codereview.appspot.com/5502059/diff/1010/src/pkg/go/doc/doc.go#newcode76
src/pkg/go/doc/doc.go:76: if name == "" || name == "_" {
On 2011/12/22 20:30:08, rsc wrote:
> Is this a type name?  I don't think you can encounter _ as a type name
> so I am a little confused.

It is. If an unfiltered AST (including non-exported stuff) is go/doc-ed, it's
possible to see a _ type:

type _ struct {...}

No need to keep information about it.

http://codereview.appspot.com/5502059/diff/1010/src/pkg/go/doc/doc.go#newcode403
src/pkg/go/doc/doc.go:403: // TODO(gri) Should sort methods also by ptr vs
non-ptr receiver.
On 2011/12/22 20:30:08, rsc wrote:
> I suspect that would be a mistake, because it would be
> alphabetical in so many cases that people would assume
> it was alphabetical instead of being two runs of alphabetical
> concatenated.  That is, if it looks alphabetical most of the time,
> it should be alphabetical all of the time.
> 
> There are file systems with large block sizes that keep their
> directories sorted within a block but not across blocks, and
> I've seen bugs where people assume that the directory entries
> being read off disk are sorted, because in small directories
> (which fit in a single disk block) they are.
> 

Done.

[gri, 2011-12-22 21:11:46]

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

go/doc, godoc: show methods of anonymous fields

Missing: Handling of embedded interfaces.

Also, for reasons outlined in the previous CL (5500055), embedded
types have to be exported for its "inherited" methods to be visible.
This will be addressed w/ a subsequent CL.

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

[rsc, 2011-12-22 21:32:34]

On Thu, Dec 22, 2011 at 16:06,  <gri@golang.org> wrote:
> type _ struct {...}
>
> No need to keep information about it.

Aha.  I was thinking this name came out of a
field definition in a struct.