x/tools/godoc: Anchors for variables and constants no longer added #19894

Kapeli · 2017-04-08T11:16:22Z

Before 1.8.1, I was able to go to https://golang.org/pkg/archive/tar/#TypeBlock and it would scroll to the relevant constant. This no longer works.

Somewhat related: this makes it impossible for Dash to index constants and variables in the Go docs.

bradfitz · 2017-04-08T16:28:31Z

There was nothing in Go 1.8.1 changing godoc.

Or do you mean before Go 1.8?

Even then, I don't think this is true. We added #anchors in Go 1.8 but didn't change or remove any.

Kapeli · 2017-04-08T16:36:54Z

Ah, you're right. The change seems to have happened with the 1.8 release, not 1.8.1.

If you go to this snapshot from Feb 2017 (Go 1.7.5) you'll be able to jump to the TypeBlock constant: https://web.archive.org/web/20170211005446/https://golang.org/pkg/archive/tar/#TypeBlock.

This doesn't work with the live version anymore: https://golang.org/pkg/archive/tar/#TypeBlock.

Notice the span id in the screenshot below:

There's no more span id in the latest docs:

bradfitz · 2017-04-08T17:22:50Z

Hm, strange.

It'd be nice if somebody could bisect (or manually review godoc commits--- there aren't many) and figure out when we (I?) broke it.

/cc @odeke-em @shurcooL

dmitshur · 2017-04-09T03:21:43Z

I bisected x/tools/cmd/godoc.

The problem starts with golang/tools@e1bdc76.

With the parent commit, the generated HTML looks like this:

With golang/tools@e1bdc76, you get:

I'm not up to date with how cmd/godoc handles anchor links in the latest version. I recall seeing some changes that pushed their generation on the frontend via JavaScript. So at this point, I'm not sure if the anchor is supposed to be generated on backend or frontend. I might be misremembering this (perhaps that was for other pages of golang.org, not /pkg/ pages).

/cc @jayconrod

dmitshur · 2017-04-09T03:32:31Z

Here's a diff of the generated HTML at localhost:6060/pkg/archive/tar/:

@@ -333,40 +333,40 @@
 				<p>
 Header type flags.
 </p>
 
 				<pre>const (
-    <span id="TypeReg">TypeReg</span>           = &#39;0&#39;    <span class="comment">// regular file</span>
-    <span id="TypeRegA">TypeRegA</span>          = &#39;\x00&#39; <span class="comment">// regular file</span>
-    <span id="TypeLink">TypeLink</span>          = &#39;1&#39;    <span class="comment">// hard link</span>
-    <span id="TypeSymlink">TypeSymlink</span>       = &#39;2&#39;    <span class="comment">// symbolic link</span>
-    <span id="TypeChar">TypeChar</span>          = &#39;3&#39;    <span class="comment">// character device node</span>
-    <span id="TypeBlock">TypeBlock</span>         = &#39;4&#39;    <span class="comment">// block device node</span>
-    <span id="TypeDir">TypeDir</span>           = &#39;5&#39;    <span class="comment">// directory</span>
-    <span id="TypeFifo">TypeFifo</span>          = &#39;6&#39;    <span class="comment">// fifo node</span>
-    <span id="TypeCont">TypeCont</span>          = &#39;7&#39;    <span class="comment">// reserved</span>
-    <span id="TypeXHeader">TypeXHeader</span>       = &#39;x&#39;    <span class="comment">// extended header</span>
-    <span id="TypeXGlobalHeader">TypeXGlobalHeader</span> = &#39;g&#39;    <span class="comment">// global extended header</span>
-    <span id="TypeGNULongName">TypeGNULongName</span>   = &#39;L&#39;    <span class="comment">// Next file has a long name</span>
-    <span id="TypeGNULongLink">TypeGNULongLink</span>   = &#39;K&#39;    <span class="comment">// Next file symlinks to a file w/ a long name</span>
-    <span id="TypeGNUSparse">TypeGNUSparse</span>     = &#39;S&#39;    <span class="comment">// sparse file</span>
+    TypeReg           = &#39;0&#39;    <span class="comment">// regular file</span>
+    TypeRegA          = &#39;\x00&#39; <span class="comment">// regular file</span>
+    TypeLink          = &#39;1&#39;    <span class="comment">// hard link</span>
+    TypeSymlink       = &#39;2&#39;    <span class="comment">// symbolic link</span>
+    TypeChar          = &#39;3&#39;    <span class="comment">// character device node</span>
+    TypeBlock         = &#39;4&#39;    <span class="comment">// block device node</span>
+    TypeDir           = &#39;5&#39;    <span class="comment">// directory</span>
+    TypeFifo          = &#39;6&#39;    <span class="comment">// fifo node</span>
+    TypeCont          = &#39;7&#39;    <span class="comment">// reserved</span>
+    TypeXHeader       = &#39;x&#39;    <span class="comment">// extended header</span>
+    TypeXGlobalHeader = &#39;g&#39;    <span class="comment">// global extended header</span>
+    TypeGNULongName   = &#39;L&#39;    <span class="comment">// Next file has a long name</span>
+    TypeGNULongLink   = &#39;K&#39;    <span class="comment">// Next file symlinks to a file w/ a long name</span>
+    TypeGNUSparse     = &#39;S&#39;    <span class="comment">// sparse file</span>
 )</pre>
 
 
 
 			<h2 id="pkg-variables">Variables</h2>
 
 
 				<pre>var (
-    <span id="ErrWriteTooLong">ErrWriteTooLong</span>    = <a href="/pkg/errors/">errors</a>.<a href="/pkg/errors/#New">New</a>(&#34;archive/tar: write too long&#34;)
-    <span id="ErrFieldTooLong">ErrFieldTooLong</span>    = <a href="/pkg/errors/">errors</a>.<a href="/pkg/errors/#New">New</a>(&#34;archive/tar: header field too long&#34;)
-    <span id="ErrWriteAfterClose">ErrWriteAfterClose</span> = <a href="/pkg/errors/">errors</a>.<a href="/pkg/errors/#New">New</a>(&#34;archive/tar: write after close&#34;)
+    ErrWriteTooLong    = <a href="/pkg/errors/">errors</a>.<a href="/pkg/errors/#New">New</a>(&#34;archive/tar: write too long&#34;)
+    ErrFieldTooLong    = <a href="/pkg/errors/">errors</a>.<a href="/pkg/errors/#New">New</a>(&#34;archive/tar: header field too long&#34;)
+    ErrWriteAfterClose = <a href="/pkg/errors/">errors</a>.<a href="/pkg/errors/#New">New</a>(&#34;archive/tar: write after close&#34;)
 )</pre>
 
 
 				<pre>var (
-    <span id="ErrHeader">ErrHeader</span> = <a href="/pkg/errors/">errors</a>.<a href="/pkg/errors/#New">New</a>(&#34;archive/tar: invalid tar header&#34;)
+    ErrHeader = <a href="/pkg/errors/">errors</a>.<a href="/pkg/errors/#New">New</a>(&#34;archive/tar: invalid tar header&#34;)
 )</pre>
 
 
 
 
@@ -409,11 +409,11 @@
 
 
 				<h3 id="FileInfoHeader">func <a href="/src/archive/tar/common.go?s=5687:5752#L189">FileInfoHeader</a>
 					<a class="permalink" href="#FileInfoHeader">&#xb6;</a>
 				</h3>
-				<pre>func FileInfoHeader(fi <a href="/pkg/os/">os</a>.<a href="/pkg/os/#FileInfo">FileInfo</a>, link <a href="/pkg/builtin/#string">string</a>) (*<a href="#Header">Header</a>, <a href="/pkg/builtin/#error">error</a>)</pre>
+				<pre>func <a href="#FileInfoHeader">FileInfoHeader</a>(fi <a href="/pkg/os/">os</a>.<a href="/pkg/os/#FileInfo">FileInfo</a>, link <a href="/pkg/builtin/#string">string</a>) (*<a href="#Header">Header</a>, <a href="/pkg/builtin/#error">error</a>)</pre>
 				<p>
 FileInfoHeader creates a partially-populated Header from fi.
 If fi describes a symlink, FileInfoHeader records link as the link target.
 If fi describes a directory, a slash is appended to the name.
 Because os.FileInfo&#39;s Name method returns only the base name of
@@ -428,11 +428,11 @@
 
 
 				<h3 id="Header.FileInfo">func (*Header) <a href="/src/archive/tar/common.go?s=2353:2392#L56">FileInfo</a>
 					<a class="permalink" href="#Header.FileInfo">&#xb6;</a>
 				</h3>
-				<pre>func (h *<a href="#Header">Header</a>) FileInfo() <a href="/pkg/os/">os</a>.<a href="/pkg/os/#FileInfo">FileInfo</a></pre>
+				<pre>func (h *<a href="#Header">Header</a>) <a href="#FileInfo">FileInfo</a>() <a href="/pkg/os/">os</a>.<a href="/pkg/os/#FileInfo">FileInfo</a></pre>
 				<p>
 FileInfo returns an os.FileInfo for the Header.
 </p>
 
 
@@ -467,11 +467,11 @@
 
 
 				<h3 id="NewReader">func <a href="/src/archive/tar/reader.go?s=3242:3277#L90">NewReader</a>
 					<a class="permalink" href="#NewReader">&#xb6;</a>
 				</h3>
-				<pre>func NewReader(r <a href="/pkg/io/">io</a>.<a href="/pkg/io/#Reader">Reader</a>) *<a href="#Reader">Reader</a></pre>
+				<pre>func <a href="#NewReader">NewReader</a>(r <a href="/pkg/io/">io</a>.<a href="/pkg/io/#Reader">Reader</a>) *<a href="#Reader">Reader</a></pre>
 				<p>
 NewReader creates a new Reader reading from r.
 </p>
 
 
@@ -481,11 +481,11 @@
 
 
 				<h3 id="Reader.Next">func (*Reader) <a href="/src/archive/tar/reader.go?s=3409:3450#L95">Next</a>
 					<a class="permalink" href="#Reader.Next">&#xb6;</a>
 				</h3>
-				<pre>func (tr *<a href="#Reader">Reader</a>) Next() (*<a href="#Header">Header</a>, <a href="/pkg/builtin/#error">error</a>)</pre>
+				<pre>func (tr *<a href="#Reader">Reader</a>) <a href="#Next">Next</a>() (*<a href="#Header">Header</a>, <a href="/pkg/builtin/#error">error</a>)</pre>
 				<p>
 Next advances to the next entry in the tar archive.
 </p>
 <p>
 io.EOF is returned at the end of the input.
@@ -497,11 +497,11 @@
 
 
 				<h3 id="Reader.Read">func (*Reader) <a href="/src/archive/tar/reader.go?s=22332:22377#L685">Read</a>
 					<a class="permalink" href="#Reader.Read">&#xb6;</a>
 				</h3>
-				<pre>func (tr *<a href="#Reader">Reader</a>) Read(b []<a href="/pkg/builtin/#byte">byte</a>) (<a href="/pkg/builtin/#int">int</a>, <a href="/pkg/builtin/#error">error</a>)</pre>
+				<pre>func (tr *<a href="#Reader">Reader</a>) <a href="#Read">Read</a>(b []<a href="/pkg/builtin/#byte">byte</a>) (<a href="/pkg/builtin/#int">int</a>, <a href="/pkg/builtin/#error">error</a>)</pre>
 				<p>
 Read reads from the current entry in the tar archive.
 It returns 0, io.EOF when it reaches the end of that entry,
 until Next is called to advance to the next entry.
 </p>
@@ -543,11 +543,11 @@
 
 
 				<h3 id="NewWriter">func <a href="/src/archive/tar/writer.go?s=1468:1503#L36">NewWriter</a>
 					<a class="permalink" href="#NewWriter">&#xb6;</a>
 				</h3>
-				<pre>func NewWriter(w <a href="/pkg/io/">io</a>.<a href="/pkg/io/#Writer">Writer</a>) *<a href="#Writer">Writer</a></pre>
+				<pre>func <a href="#NewWriter">NewWriter</a>(w <a href="/pkg/io/">io</a>.<a href="/pkg/io/#Writer">Writer</a>) *<a href="#Writer">Writer</a></pre>
 				<p>
 NewWriter creates a new Writer writing to w.
 </p>
 
 
@@ -557,11 +557,11 @@
 
 
 				<h3 id="Writer.Close">func (*Writer) <a href="/src/archive/tar/writer.go?s=10589:10620#L336">Close</a>
 					<a class="permalink" href="#Writer.Close">&#xb6;</a>
 				</h3>
-				<pre>func (tw *<a href="#Writer">Writer</a>) Close() <a href="/pkg/builtin/#error">error</a></pre>
+				<pre>func (tw *<a href="#Writer">Writer</a>) <a href="#Close">Close</a>() <a href="/pkg/builtin/#error">error</a></pre>
 				<p>
 Close closes the tar archive, flushing any unwritten
 data to the underlying writer.
 </p>
 
@@ -571,11 +571,11 @@
 
 
 				<h3 id="Writer.Flush">func (*Writer) <a href="/src/archive/tar/writer.go?s=1585:1616#L39">Flush</a>
 					<a class="permalink" href="#Writer.Flush">&#xb6;</a>
 				</h3>
-				<pre>func (tw *<a href="#Writer">Writer</a>) Flush() <a href="/pkg/builtin/#error">error</a></pre>
+				<pre>func (tw *<a href="#Writer">Writer</a>) <a href="#Flush">Flush</a>() <a href="/pkg/builtin/#error">error</a></pre>
 				<p>
 Flush finishes writing the current file (optional).
 </p>
 
 
@@ -584,11 +584,11 @@
 
 
 				<h3 id="Writer.Write">func (*Writer) <a href="/src/archive/tar/writer.go?s=10168:10220#L314">Write</a>
 					<a class="permalink" href="#Writer.Write">&#xb6;</a>
 				</h3>
-				<pre>func (tw *<a href="#Writer">Writer</a>) Write(b []<a href="/pkg/builtin/#byte">byte</a>) (n <a href="/pkg/builtin/#int">int</a>, err <a href="/pkg/builtin/#error">error</a>)</pre>
+				<pre>func (tw *<a href="#Writer">Writer</a>) <a href="#Write">Write</a>(b []<a href="/pkg/builtin/#byte">byte</a>) (n <a href="/pkg/builtin/#int">int</a>, err <a href="/pkg/builtin/#error">error</a>)</pre>
 				<p>
 Write writes to the current entry in the tar archive.
 Write returns the error ErrWriteTooLong if more than
 hdr.Size bytes are written after WriteHeader.
 </p>
@@ -599,11 +599,11 @@
 
 
 				<h3 id="Writer.WriteHeader">func (*Writer) <a href="/src/archive/tar/writer.go?s=2275:2323#L69">WriteHeader</a>
 					<a class="permalink" href="#Writer.WriteHeader">&#xb6;</a>
 				</h3>
-				<pre>func (tw *<a href="#Writer">Writer</a>) WriteHeader(hdr *<a href="#Header">Header</a>) <a href="/pkg/builtin/#error">error</a></pre>
+				<pre>func (tw *<a href="#Writer">Writer</a>) <a href="#WriteHeader">WriteHeader</a>(hdr *<a href="#Header">Header</a>) <a href="/pkg/builtin/#error">error</a></pre>
 				<p>
 WriteHeader writes hdr and prepares to accept the file&#39;s contents.
 WriteHeader calls Flush if it is not the first header.
 Calling after a Close will return ErrWriteAfterClose.
 </p>

It affects quite a few things, not just constants.

Edit: Some of the changed lines have added a hrefs, those are fine. It's the missing span ids that are the problem.

bradfitz · 2017-04-09T06:04:29Z

@shurcooL, thanks!

/cc @jayconrod @alandonovan

jayconrod · 2017-04-10T22:06:44Z

I looked into this today. After my change, linksFor generates empty links for const and var declarations. Previously, it included the identifier name. LinkifyText skips over these, since it doesn't know what id to write.

I should have a fix for this tomorrow.

bradfitz · 2017-04-10T22:29:20Z

Thanks!

gopherbot · 2017-04-11T20:31:46Z

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

bradfitz · 2017-04-14T23:26:45Z

Reopening for cherry-pick.

Kapeli · 2017-04-15T07:39:47Z

Any chance the golang.org docs could be regenerated using the fixed version of godoc?

bradfitz · 2017-04-15T14:42:45Z

@Kapeli, yes, that's what I mean by "reopening for cherry-pick".

dmitshur · 2017-04-15T20:42:41Z

@bradfitz, you said "reopening", but the issue continues to be closed. Is there a different meaning to the word "reopening", or did you forget to actually reopen it?

bradfitz · 2017-04-15T22:56:27Z

Forgot.

broady · 2017-05-23T20:23:00Z

Cherry-pick doesn't apply cleanly. I've asked @jayconrod to take a look.

dmitshur · 2017-05-23T20:31:03Z

Cherry-pick doesn't apply cleanly.

golang/tools@3bcb6ef (the commit that closed this issue) is a minor patch/bug fix for golang/tools@e1bdc76, where this regression was introduced. But the corresponding issue for that commit is milestoned for Go 1.9. If it's not a part of 1.8 release, then this fixup shouldn't be needed either.

bradfitz · 2017-05-23T20:33:37Z

@shurcooL, thanks!

jayconrod · 2017-05-23T20:34:14Z

Confirming what @shurcooL said, this bug is not on the 1.8 branch and does not need to be cherry-picked.

gopherbot added this to the Unreleased milestone Apr 8, 2017

bradfitz added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Apr 8, 2017

bradfitz added help wanted and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Apr 8, 2017

bradfitz modified the milestones: Go1.8.2, Unreleased Apr 9, 2017

jayconrod self-assigned this Apr 10, 2017

dlespiau mentioned this issue Apr 11, 2017

cmd/godoc: var anchors are misplaced #19136

Closed

gopherbot closed this as completed in golang/tools@3bcb6ef Apr 14, 2017

bradfitz reopened this Apr 15, 2017

dmitshur mentioned this issue May 9, 2017

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

Closed

bradfitz modified the milestones: Go1.8.2, Go1.8.3 May 18, 2017

jayconrod closed this as completed May 23, 2017

bradfitz modified the milestones: Go1.9, Go1.8.3 May 23, 2017

golang locked and limited conversation to collaborators May 23, 2018

gopherbot added the FrozenDueToAge label May 23, 2018

rsc unassigned jayconrod Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/godoc: Anchors for variables and constants no longer added #19894

x/tools/godoc: Anchors for variables and constants no longer added #19894

Kapeli commented Apr 8, 2017

bradfitz commented Apr 8, 2017

Kapeli commented Apr 8, 2017

bradfitz commented Apr 8, 2017

dmitshur commented Apr 9, 2017 •

edited

dmitshur commented Apr 9, 2017 •

edited

bradfitz commented Apr 9, 2017

jayconrod commented Apr 10, 2017

bradfitz commented Apr 10, 2017

gopherbot commented Apr 11, 2017

bradfitz commented Apr 14, 2017

Kapeli commented Apr 15, 2017

bradfitz commented Apr 15, 2017

dmitshur commented Apr 15, 2017

bradfitz commented Apr 15, 2017

broady commented May 23, 2017

dmitshur commented May 23, 2017 •

edited

bradfitz commented May 23, 2017

jayconrod commented May 23, 2017 •

edited

x/tools/godoc: Anchors for variables and constants no longer added #19894

x/tools/godoc: Anchors for variables and constants no longer added #19894

Comments

Kapeli commented Apr 8, 2017

bradfitz commented Apr 8, 2017

Kapeli commented Apr 8, 2017

bradfitz commented Apr 8, 2017

dmitshur commented Apr 9, 2017 • edited

dmitshur commented Apr 9, 2017 • edited

bradfitz commented Apr 9, 2017

jayconrod commented Apr 10, 2017

bradfitz commented Apr 10, 2017

gopherbot commented Apr 11, 2017

bradfitz commented Apr 14, 2017

Kapeli commented Apr 15, 2017

bradfitz commented Apr 15, 2017

dmitshur commented Apr 15, 2017

bradfitz commented Apr 15, 2017

broady commented May 23, 2017

dmitshur commented May 23, 2017 • edited

bradfitz commented May 23, 2017

jayconrod commented May 23, 2017 • edited

dmitshur commented Apr 9, 2017 •

edited

dmitshur commented Apr 9, 2017 •

edited

dmitshur commented May 23, 2017 •

edited

jayconrod commented May 23, 2017 •

edited