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

Jump to bottom

x/tools/godoc: Remove the function declaration link #20269

Closed
178inaba opened this issue May 6, 2017 · 3 comments
Closed

x/tools/godoc: Remove the function declaration link #20269

178inaba opened this issue May 6, 2017 · 3 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@178inaba
Copy link
Contributor

178inaba commented May 6, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.8.1 darwin/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/i178inaba/work/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.8.1/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.8.1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/8b/5j9t64vs0sg_n1jjfkkrg88m0000gn/T/go-build329351097=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

I referred to the following documents.

https://golang.org/pkg/net/http/#Client.Get
https://golang.org/pkg/net/http/#Header.Get

What did you expect to see?

The Fragment link should be as follows.

#Client.Get
#Header.Get

<pre>func (c *<a href="#Client">Client</a>) <a href="#Client.Get">Get</a>(url <a href="/pkg/builtin/#string">string</a>) (resp *<a href="#Response">Response</a>, err <a href="/pkg/builtin/#error">error</a>)</pre>
<pre>func (h <a href="#Header">Header</a>) <a href="#Header.Get">Get</a>(key <a href="/pkg/builtin/#string">string</a>) <a href="/pkg/builtin/#string">string</a></pre>

What did you see instead?

The function name link of the part displaying the signature of the method is invalid.

<pre>func (c *<a href="#Client">Client</a>) <a href="#Get">Get</a>(url <a href="/pkg/builtin/#string">string</a>) (resp *<a href="#Response">Response</a>, err <a href="/pkg/builtin/#error">error</a>)</pre>
<pre>func (h <a href="#Header">Header</a>) <a href="#Get">Get</a>(key <a href="/pkg/builtin/#string">string</a>) <a href="/pkg/builtin/#string">string</a></pre>
@gopherbot gopherbot added this to the Unreleased milestone May 6, 2017
@gopherbot
Copy link

CL https://golang.org/cl/42892 mentions this issue.

@178inaba 178inaba changed the title x/tools/cmd/godoc: Method's Fragment link is invalid x/tools/godoc: Method's Fragment link is invalid May 8, 2017
@dmitshur
Copy link
Contributor

dmitshur commented May 9, 2017

I've investigated this. This issue is actually 2 issues, and the correct fix is different from what's suggested above. One is a deploy/release issue (what's running at golang.org), and the other is a code issue in latest master of tools subrepo. Both need to be resolved.

The version of cmd/godoc that's running at http://golang.org/pkg/ is weird. I can't easily tell what version exactly it is, but I can tell it's not the version that's a part of go1.8.1 release (which didn't have this bug), and it's not the latest master of tools subrepo (which has this bug, but the fix is different from what the issue suggests). So, I'm going to ignore it below and discuss what I can reproduce locally with latest master version of cmd/godoc in tools subrepo.

This bug was actually introduced in golang/tools@e1bdc76 (similar to #19894 (comment)). /cc @jayconrod

With the parent commit (golang/tools@00f7cd5), the generated HTML looked like this:

image

With golang/tools@e1bdc76, it becomes:

image

Notice that the Get method name was not linked before, but it becomes linked after. And the link is wrong.

So, the correct fix is to remove the link, not to fix the URL. The link wasn't meant to be added, it was an unintentional regression caused by golang/tools@e1bdc76. Its commit message confirms it was meant to be a fix for composite literals anchors only.

(The link doesn't make sense in this context, because it points to the same location as the function/method. There's already a heading that does that too. That's why it should be removed.)

@178inaba
Copy link
Contributor Author

178inaba commented May 9, 2017

Thank you so much for review!
I'm going to fix it according to your review.

@178inaba 178inaba changed the title x/tools/godoc: Method's Fragment link is invalid x/tools/godoc: Delete the function declaration link May 10, 2017
@178inaba 178inaba changed the title x/tools/godoc: Delete the function declaration link x/tools/godoc: Remove the function declaration link May 10, 2017
@golang golang locked and limited conversation to collaborators May 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@dmitshur @gopherbot @178inaba