golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.84k stars 17.51k forks source link

proposal: x/tools/{godoc,cmd/godoc}: isolate, tag, and delete godoc #59056

Open adonovan opened 1 year ago

adonovan commented 1 year ago

The godoc command, golang.org/x/tools/cmd/godoc, was never redesigned to make full use of modules, has been deprecated in favor of pkgsite (https://pkg.go.dev), and has been neglected for some time. We propose to isolate, tag, and delete it from the golang.org/x/tools repository, using a similar process to the deletion of cmd/cover.

The main objection to doing so was that godoc is still useful for previewing the documentation of locally edited code, and that its replacement, pkgsite, was not well suited to the local preview use case and really needs additional features such as the ability to run without the network and to load packages on demand after startup. However, @findleyr recently added those features.

Are there any remaining reasons why we should not tag and delete cmd/godoc?

Related:

aarzilli commented 1 year ago

I don't like that the recommended way to see documentation locally is something with 43 direct dependencies and 57 indirect dependencies. Why does it need two database drivers (redis and postgres), a VM for a scripting language (lua), an implementation of SSH, two RPC layers (grpc and protobuf), oauth2 and bindings to OpenTelemetry?

Pkgsite starts slower, serves pages slower, takes 30 seconds to load a standard library package every time I start it and it doesn't have the convenient list of packages that godoc has.

zigo101 commented 1 year ago

@aarzilli You may try Golds for an alternative.

findleyr commented 1 year ago

Pkgsite starts slower, serves pages slower, takes 30 seconds to load a standard library package every time I start it and it doesn't have the convenient list of packages that godoc has.

@aarzilli FWIW I plan to fix both the slow loading and the downloading of the stdlib as part of https://go.dev/issue/40371. The slowness is due to rendering the entire module at once (rather than rendering packages as they are accessed). The stdlib loading time is due to downloading the stdlib rather than rendering GOROOT. Both should be fixable, and are definitely worth fixing.

As for why it needs so many dependencies: it is in the same module as pkg.go.dev, which integrates with cloud services. I suppose we could avoid this by splitting x/pkgsite into multiple modules, but that seems like a non-trivial amount of complexity and maintenance burden for little gain. I could be convinced otherwise, if folks have strong reasons why this is a problem.

aarzilli commented 1 year ago

That's what I'm saying, there's always been two programs, what pkgsite is now used to be called gddo, it's not worth the effort trying to make pkgsite work well for both. The maintenance burden of godoc has been small so far, it should just be undeprecated.

findleyr commented 1 year ago

I agree that the maintenance burden has been light thus far, and really appreciate all the community members that have stepped in to help with godoc. But having godoc live in x/tools means that it is ultimately the responsibility of the Go team to (at the very least) review CLs and not break the build. Ultimately, this requires attention, and attention is an extremely valuable resource!

Furthermore, there is no guarantee that the burden of this responsibility will continue to be light in the future. If in the future some change in Go causes godoc tests to fail, or if some critical bug is discovered in godoc, it will be incumbent upon the Go team to address the problem quickly, even if the fix is only to e.g. skip the test. (and if that's the fix, isn't it clearer to remove godoc from x/tools so as to not imply a level of support?).

aarzilli commented 1 year ago

If that were to happen it could be removed then. On what basis do you predict the burden of maintenance will increase?

amery commented 1 year ago

@aarzilli You may try Golds for an alternative.

I tried @go101, but it falls apart when you work on multiple modules :sob:

and pkgsite is still useless for local development. godoc seems to be still the only viable option

zigo101 commented 1 year ago

Yes, as Golds provides some richer features, such as showing be-direction relations between deping and deped packages (such as type implementation relation), there are some difficulties to parse multiple versions of a package at the same time, which is often caused by parsing multiple seed modules.

I believe the limitation originates from the golang.org/x/tools/go/packages package, though it is possible to make (some heavy) efforts to let Golds work through the limitation.

zhsj commented 1 year ago

What's the recommended alternative for rendering private modules?

amery commented 1 year ago

Yes, as Golds provides some richer features, such as showing be-direction relations between deping and deped packages (such as type implementation relation), there are some difficulties to parse multiple versions of a package at the same time, which is often caused by parsing multiple seed modules.

I believe the limitation originates from the golang.org/x/tools/go/packages package, though it is possible to make (some heavy) efforts to let Golds work through the limitation.

wouldn't the dependency graph sort that instead of assuming the first package is the leaf?

adonovan commented 1 year ago

[@amery] pkgsite is still useless for local development.

Could you elaborate? As of last week, pkgsite should display docs reflecting locally edited packages each time you reload the page.

amery commented 1 year ago

[@amery] pkgsite is still useless for local development.

Could you elaborate? As of last week, pkgsite should display docs reflecting locally edited packages each time you reload the page.

if you run pkgsite as-is you don't get any CSS or image, and to run frontend you need postgresql. pretty high requirements just to see the generated docs as godoc does out of the box

findleyr commented 1 year ago

@amery please run x/pkgsite/cmd/pkgsite, which is the local version of pkgsite.

Edit: more specifically:

Install:

$ go install golang.org/x/pkgsite/cmd/pkgsite@master

Run:

$ pkgsite
amery commented 1 year ago

@amery please run x/pkgsite/cmd/pkgsite, which is the local version of pkgsite.

Edit: more specifically:

Install:

$ go install golang.org/x/pkgsite/cmd/pkgsite@master

Run:

$ pkgsite

I can confirm it now works. no idea why yesterday it was getting 404 from all static content pulled by the template. thanks a lot @findleyr

fzipp commented 1 year ago

Some of the texts on the website mentioning godoc should probably be revised:

Finally the fmt and godoc commands are installed as regular binaries called gofmt and godoc because they are so often referenced.

--> godoc hasn't been included in the Go distribution since Go 1.13

--> the website (now go.dev) is no longer served by godoc

--> not wrong, but draws unnecessary attention to godoc, which is deprecated

qiulaidongfeng commented 1 year ago

There are also several reasons not to delete cmd/godoc

mvdan commented 1 year ago

@qiulaidongfeng note that you could always continue to use the last tag for godoc, as described in the original post. It won't get updates, but godoc isn't currently getting updates in master either.

adonovan commented 1 year ago

@qiulaidongfeng I've reported #60114 for the slow std download problem and https://github.com/golang/go/issues/40371#issuecomment-1542535193 for the unexported symbols feature.

gopherbot commented 10 months ago

Change https://go.dev/cl/541237 mentions this issue: gopls/internal/lsp: add "Browse package documentation" code action

qiulaidongfeng commented 7 months ago

Find another reason not to delete cmd/godoc.

See #64920 and #61976 , If my use godoc, my experience is don't have to use the Internet.

More importantly, if my CL is used to modify documentation, in cmd, type godoc and you will see the revised std documentation in your browser. Note that this practice relies on doing so that in order for vscode or gopls to work properly, it does not display hundreds of errors (e.g. use of internal package internal/abi not allowed), used in cmd ,set GOROOT=git clone go directory

See https://github.com/golang/go/issues/60114#issuecomment-1901474945 , use pkgsite can't see.

adonovan commented 7 months ago

I believe the current consensus on the plan is for go doc -http [symbol] to start a local pkgsite instance which, thanks to work underway by @matloob, will start quickly and quietly. With a symbol argument, it will open a browser at the correct localhost URL. The details still need to be worked out.

In other news, VS Code is likely to get client-side support for rendering documentation within a pane of VS Code itself; see https://github.com/golang/go/issues/64936#issuecomment-1874762262. Other LSP clients will open a browser connected to a local pkgsite instance, or, perhaps, to the gopls server itself, which may serve pkgsite-like documentation; again there are details to work out.

gopherbot commented 6 months ago

Change https://go.dev/cl/571101 mentions this issue: _content/doc: update references to obsolete godoc

adonovan commented 2 months ago

Other LSP clients will open a browser connected to a local pkgsite instance, or, perhaps, to the gopls server itself, which may serve pkgsite-like documentation; again there are details to work out.

gopls/v0.16.0, released today, contains an integrated documentation viewer, similar in style to pkg.go.dev, but that works on local unpublished packages and even unsaved editor buffers.

The remaining blocker for this issue is to make go doc -http work.

coxley commented 2 months ago

@adonovan Love the idea with the new update, but question on this:

contains an integrated documentation viewer, similar in style to pkg.go.dev

The style feels closer to the old godoc than it does pkg.go.dev to me — mostly because the content fills the viewport. On a large screen, reading is tough.

Has it been discussed to at least have similar stylesheets to pkgsite? Content is centered, 80% of the viewport width with 20% side gutters, one for navigation. The select-dropdown for nav feels a bit clunky to use.

(Not a huge criticism by any means, still love the update :)

3052 commented 2 months ago

it seems the HTML option is being ignored. people need a simple way to dump a module or package as HTML files.

adonovan commented 2 months ago

Has it been discussed to at least have similar stylesheets to pkgsite? Content is centered, 80% of the viewport width with 20% side gutters, one for navigation.

Yes, I think that would be a definite improvement; it's simply been a matter of time/effort. We'll get there eventually, and would be happy to accept contributions.