Skip to content

proposal: cmd/godoc: treat function returning slice as constructor #18063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mvdan opened this issue Nov 27, 2016 · 14 comments
Closed

proposal: cmd/godoc: treat function returning slice as constructor #18063

mvdan opened this issue Nov 27, 2016 · 14 comments

Comments

@mvdan
Copy link
Member

mvdan commented Nov 27, 2016

This is sort of a follow-up to #14004. Sometimes, there are constructors that return a slice of a type. For example:

https://golang.org/pkg/net/mail/#pkg-index

func ParseAddressList(list string) ([]*Address, error)
type Address
    func ParseAddress(address string) (*Address, error)
    func (a *Address) String() string

go/doc does not consider ParseAddressList an Address type func as it returns a slice, not the type directly. Since this rule is reserved for the first returned type, I suggest this is relaxed to also allow slices (and perhaps arrays too? but couldn't find an example of that).

CC @aclements @robpike

@dsnet
Copy link
Member

dsnet commented Nov 27, 2016

Before considering whether the go doc command-line tool should do this, I think we should first discuss of whether the godoc webserver should do this. The link you provided seems to indicate that godoc currently doesn't do this.

There may be strange edge-cases, but grouping slices and arrays of a given type seems reasonable to me.

@mvdan
Copy link
Member Author

mvdan commented Nov 28, 2016

Right - even after a few years I still get confused between go doc and godoc. I did indeed mean godoc.

Before considering whether the go doc command-line tool should do this, I think we should first discuss of whether the godoc webserver should do this.

Out of curiosity, why is that? I would assume the logic behind the two - at least the logic concerning constructor funcs - would be the same.

@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 29, 2016
@quentinmit quentinmit added this to the Go1.9 milestone Nov 29, 2016
@dsnet
Copy link
Member

dsnet commented Dec 7, 2016

Out of curiosity, why is that? I would assume the logic behind the two - at least the logic concerning constructor funcs - would be the same.

As of late, the behavior of go doc has been to follow whatever godoc does. It's reasonable to assume that they would be the same. It's just that the change has to happen on one of them first.

@minux
Copy link
Member

minux commented Dec 7, 2016 via email

@mvdan mvdan changed the title go/doc: also group slice constructors x/tools/cmd/godoc: also group slice constructors Apr 26, 2017
@mvdan
Copy link
Member Author

mvdan commented Apr 26, 2017

we need to consider how far do we want to get into reordering.

This proposal is limited to just slices whose element types are already grouped as constructors. So if funcs that return Foo and *Foo are grouped, those that return []Foo and []*Foo should be too.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jun 7, 2017
@mvdan mvdan changed the title x/tools/cmd/godoc: also group slice constructors Proposal: x/tools/cmd/godoc: also group slice constructors Jun 16, 2017
@mvdan mvdan modified the milestones: Proposal, Go1.10 Jun 16, 2017
@mvdan mvdan added Proposal and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 16, 2017
@mvdan
Copy link
Member Author

mvdan commented Jun 16, 2017

Not sure why I didn't make this a proposal from the start; made it one.

@rsc rsc changed the title Proposal: x/tools/cmd/godoc: also group slice constructors proposal: cmd/godoc: treat function returning slice as constructor Jun 26, 2017
@rsc
Copy link
Contributor

rsc commented Jun 26, 2017

/cc @griesemer

@rsc
Copy link
Contributor

rsc commented Jul 17, 2017

These would be the "new" constructors in the standard library:

go1.txt:pkg crypto/x509, func ParseCertificates([]uint8) ([]*Certificate, error)
go1.txt:pkg go/doc, func Examples(...*ast.File) []*Example
go1.txt:pkg net, func LookupMX(string) ([]*MX, error)
go1.1.txt:pkg net, func LookupNS(string) ([]*NS, error)
go1.1.txt:pkg net/mail, func ParseAddressList(string) ([]*Address, error)
go1.txt:pkg runtime/pprof, func Profiles() []*Profile

At least four of these look like actual constructors. LookupMX and LookupNS are arguably not. (Of course, user.LookupId is considered a constructor for *user.User, so capturing those would be consistent at least.)

(There are a bunch of methods returning slices of things in the same package too, but methods are never "constructors".)

It does seem like it's probably reasonable.

@rsc
Copy link
Contributor

rsc commented Jul 17, 2017

accepting for @golang/proposal-review. If anyone is interested to work on this, a CL would be welcome.

@mvdan
Copy link
Member Author

mvdan commented Jul 17, 2017

Happy to work on a patch for 1.10.

@mvdan mvdan self-assigned this Jul 17, 2017
@mvdan
Copy link
Member Author

mvdan commented Jul 18, 2017

@rsc by the way, why was the title changed to remove x/tools/ from x/tools/cmd/godoc? Is that abbreviation normal for proposal issues, or does this mean that the patch should land in go doc first?

@tomwans
Copy link
Contributor

tomwans commented Aug 10, 2017

Hey @mvdan are you still working on this? I was inspired by the recent contribution workshop blog post (https://blog.golang.org/contributor-workshop) and went hunting for bugs. I have a change working locally, would you mind if I submitted a CL?

@mvdan
Copy link
Member Author

mvdan commented Aug 12, 2017

@tomwans nope, I was going to get to this after the holidays. Go ahead.

@mvdan mvdan removed their assignment Aug 12, 2017
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/54971 mentions this issue: go/doc: classify function returning slice/array as constructor

@golang golang locked and limited conversation to collaborators Aug 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants