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

go/doc: do not nil out FuncBody #26835

Closed
robpike opened this issue Aug 7, 2018 · 3 comments
Closed

go/doc: do not nil out FuncBody #26835

robpike opened this issue Aug 7, 2018 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@robpike
Copy link
Contributor

robpike commented Aug 7, 2018

The go/doc package has references to the components of the AST that it describes. For some historical reason, though, it sets the FuncBody part of the FuncDecl to nil, which makes it hard to recover the source code associated with a declaration.

It seems that godoc use to do this of the "src" flag was not set, which makes sense. But at some point the action was moved into go/doc itself, and not protected by a flag, which means the source is never available.

As part of the go.12 release, I'd like to make cmd/doc supersede godoc's command line features entirely (can't find the issue number at the moment) and add support for examples (#26715). Both of these are made much more awkward by this property of go/doc.

Finally, as a point of order, it's rude for a package to overwrite a data structure passed to it, especially if the property is not documented.

I'd like to delete the nil-ing out of these fields (the function body is not the only one), or at least provide a mechanism to prevent it.

@griesemer griesemer added this to the Go1.12 milestone Aug 7, 2018
@robpike
Copy link
Contributor Author

robpike commented Aug 7, 2018

After talking with @gri, we will try to fix this by adding a new Mode bit called KeepSource or something like that, which will be off by default and therefore backwards-compatible.

@agnivade agnivade added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 11, 2018
@gopherbot
Copy link

Change https://golang.org/cl/140958 mentions this issue: go/doc: add new mode bit PreserveAST to control clearing of data in AST

@gopherbot
Copy link

Change https://golang.org/cl/140959 mentions this issue: cmd/doc: add a -src flag to show original source

gopherbot pushed a commit that referenced this issue Oct 10, 2018
It's long-desired but was blocked by #26835. That is now fixed, so
it's easy. When -src is off, we behave as before. But with -src
set, initialize the go/doc package to preserve the original AST and
things flow very easily.

With -src, since you're seeing inside the package source anyway it
shows unexported fields and constants: you see the original source.
But you still need -u to ask about them.

Fixes #18807

Change-Id: I473e90323b4eff0735360274dc0d2d9dba16ff8b
Reviewed-on: https://go-review.googlesource.com/c/140959
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Oct 10, 2019
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

4 participants