Skip to content
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

x/website: enable godoc analysis mode #11251

Closed
alml opened this issue Jun 17, 2015 · 15 comments
Closed

x/website: enable godoc analysis mode #11251

alml opened this issue Jun 17, 2015 · 15 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@alml
Copy link

alml commented Jun 17, 2015

It would be useful to annotate interfaces in the godoc with list of known implementers.
Use case: I want to use text/scanner package and parse some data I receive from network. While reading its documentation I encounter a reference to io.Reader: https://golang.org/pkg/text/scanner/#Scanner.Init. I click on "io.Reader", and it would be nice to know that besides of net.TCPConn this interface is also implemented by bufio.NewReader, which can be used in my case.

Caveat: number of types implementing trivial interfaces can be huge.

@minux
Copy link
Member

minux commented Jun 17, 2015 via email

@bradfitz
Copy link
Contributor

I think this bug is about having godoc's analysis mode enabled by default for golang.org

Related: #8968

/cc @alandonovan @adg

@bradfitz bradfitz changed the title "Known implementers" in godoc website: enable godoc analysis mode by default? Jun 17, 2015
@alml
Copy link
Author

alml commented Jun 18, 2015

Thanks! I missed "-analysis type" indeed. This does exactly what I asked for. And yes, it would be great to have it enabled on golang.org and godoc.org.

@minux
Copy link
Member

minux commented Jun 18, 2015 via email

@alml
Copy link
Author

alml commented Jun 18, 2015

Right, I mentioned this in the first post. It's a presentation issue, and I'm pretty sure it can be solved somehow. 1. Show "too many" if there are many implementers, possibly a href leading to a separate URL with full list. 2. Show some packages (most popular?) and ellipsis in the end. 3. Paginate the list. 4. Anything more smart.

@minux
Copy link
Member

minux commented Jun 18, 2015 via email

@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jul 11, 2015
@agnivade
Copy link
Contributor

ping @adg

@bradfitz
Copy link
Contributor

@andybons would be the owner of this now.

@bradfitz bradfitz modified the milestones: Unreleased, Go1.11 Apr 26, 2018
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 26, 2018
@bradfitz
Copy link
Contributor

(It seems worth doing, unless we run into performance problems on start-up or ridiculous memory usage. If either of those occur, punt to a later release or Unreleased.)

@bradfitz
Copy link
Contributor

I just tried it.

Few oddities:

  • it starts running compilations on packages it doesn't have built. I see it compiling C-based stuff locally. But that doesn't apply to golang.org where there will be no $GOPATH. So we can ignore this.
  • Why does it care about tests?
2018/07/24 18:11:38 Adding tests for archive/zip_test
2018/07/24 18:11:38 Adding tests for cmd/vendor/github.com/google/pprof/internal/symbolz
2018/07/24 18:11:38 Adding tests for vendor/golang_org/x/crypto/internal/chacha20
2018/07/24 18:11:38 Adding tests for debug/dwarf_test
2018/07/24 18:11:38 Adding tests for html/template
2018/07/24 18:11:38 Adding tests for image/draw
2018/07/24 18:11:38 Adding tests for image/gif
2018/07/24 18:11:38 Adding tests for cmd/go/internal/modload
2018/07/24 18:11:38 Adding tests for net/http/pprof
2018/07/24 18:11:38 Adding tests for net/url
2018/07/24 18:11:38 Adding tests for crypto/hmac
2018/07/24 18:11:38 Adding tests for cmd/internal/obj/x86
2018/07/24 18:11:38 Adding tests for hash/fnv
2018/07/24 18:11:38 Adding tests for go/internal/srcimporter
2018/07/24 18:11:38 Adding tests for cmd/cover_test
2018/07/24 18:11:38 Adding tests for net/internal/socktest_test
2018/07/24 18:11:38 Adding tests for internal/poll_test
2018/07/24 18:11:38 Adding tests for net/http/cookiejar_test
2018/07/24 18:11:38 Adding tests for cmd/api
2018/07/24 18:11:38 Adding tests for bufio_test
2018/07/24 18:11:38 Adding tests for debug/macho
2018/07/24 18:11:38 Adding tests for log
2018/07/24 18:11:38 Adding tests for os/signal_test
2018/07/24 18:11:38 Adding tests for encoding/base32
2018/07/24 18:11:38 Adding tests for vendor/golang_org/x/crypto/poly1305
2018/07/24 18:11:38 Adding tests for io/ioutil
2018/07/24 18:11:38 Adding tests for cmd/vendor/github.com/google/pprof/internal/binutils
2018/07/24 18:11:38 Adding tests for math/rand_test
2018/07/24 18:11:38 Adding tests for net/smtp
2018/07/24 18:11:38 Adding tests for mime_test
2018/07/24 18:11:38 Adding tests for syscall_test
2018/07/24 18:11:38 Adding tests for crypto/rc4
2018/07/24 18:11:38 Adding tests for crypto/rand_test
  • it should also skip vendor stuff

  • it should ignore tests

  • it links to source code instead of to the package docs for the implemented methods:

screen shot 2018-07-24 at 11 17 14 am

screen shot 2018-07-24 at 11 17 22 am

@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 Aug 1, 2018
@bradfitz
Copy link
Contributor

bradfitz commented Aug 1, 2018

Retargetting to Go 1.12.

/cc @dmitshur

@agnivade
Copy link
Contributor

agnivade commented Aug 4, 2018

Why does it care about tests?

According to this comment, it is creating a testmain package. I can see it found out some types that implements interfaces coming from test packages.

// Create a "testmain" package for each package with tests.
	for _, pkg := range prog.AllPackages() {
		if testmain := prog.CreateTestMainPackage(pkg); testmain != nil {
			log.Printf("Adding tests for %s", pkg.Pkg.Path())
		}
	}

it should also skip vendor stuff

That is easy.

it links to source code instead of to the package docs for the implemented methods:

One reason might be it also displays private methods. So it cannot link to package docs since private methods are not shown. We should probably fix this to match with ?m=all and show private methods only when this flag is set. Then we can always link to package docs.

Some closing thoughts -

So as I see - the only 2 new sections added by analysis mode are "implements" and "method-set". And we already have some open issues that try to incorporate some of that into main godoc.

We might need to give some thoughts on whether to polish the analysis mode and make it the default, thereby resolving the other open issues. Or do we slowly bring the features of analysis mode by fixing the open issues one by one. (Does that mean "analysis" mode will become redundant ?)

/cc @alandonovan , @griesemer

btw, running analysis mode takes a ridiculous amount of memory if we run it on GOPATH. Just GOROOT is fine. I had to kill the process after it consumed my entire RAM (16GB).

@nhooyr
Copy link
Contributor

nhooyr commented Mar 16, 2019

Regarding performance, I think it'd be fine if it only performed analysis within a single package for implemented by. Doesn't need to be across the entire $GOPATH.

Implements is I think the current package plus $GOROOT would be fine for now.

@agnivade
Copy link
Contributor

That should happen when godoc supports module mode. @dmitshur is working on that.

@bradfitz bradfitz modified the milestones: Go1.13, Go1.14 Apr 29, 2019
@rsc rsc removed this from the Go1.14 milestone Oct 9, 2019
@rsc rsc added this to the Backlog milestone Oct 9, 2019
@dmitshur dmitshur changed the title website: enable godoc analysis mode by default? x/website: enable godoc analysis mode Oct 21, 2019
@adonovan
Copy link
Member

Analysis support was dropped from godoc in CL 358954.

@golang golang locked and limited conversation to collaborators Apr 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests