x/tools/cmd/godoc: enable links to fields of structs

@broady

I would like to be able to link to:

https://golang.org/pkg/net/http/#Request.Cancel

And have the link go to the top of the comment immediately before the "Cancel" field in the Request struct.

Similarly, I would like to link the English docs at the bottom of Transport, after the fields:

https://golang.org/pkg/net/http/#Transport.doc

And have it go to...

"Transport is an implementation of RoundTripper ..."

Currently my only alternative links for both are quite some distance from the text I'd like people to read.

/cc @broady @adg @shurcooL

@odeke-em

/cc @odeke-em too

The Transport.doc one is pretty bad. I don't really like the solution. Maybe the doc should be above it, anyway? I'm unsure why godoc had it swapped (compared to how it is in source) in the first place.

Yes. Perhaps. There is also #16728 open for a very similar case.

Since you didn't mention it, are you aware of the existing related behavior of godoc.org (gddo project)?

https://godoc.org/net/http#Request.Cancel

This looks like a request to backport that, plus tweaks to where on the page the anchors position you.

Maybe the doc should be above it, anyway? I'm unsure why godoc had it swapped (compared to how it is in source) in the first place.

That's a very good question and one I've been pondering about for some months now. I'd love to know some insight on that from anyone familiar, but it might be outside of scope of this issue.

Is anyone already working on this issue/feat?

Nobody has said so here. Feel free to work on it.

@luigi-riefolo

Hi @luigi-riefolo. I looked into this briefly last week, but have no
plans to submit a changelist. Here are a few observations that might
save you some time in case you are new to the code, as I am:

For adding the link to doc section, have a look at
x/tools/godoc/static/package.html.
In order for changes to package.html to take effect, you will need
to re-generate the file static.go in the same directory, either by
running go generate or else by running the makestatic command
(located in the same directory).
You might consider renaming the #Transport.doc link to something
like #Transport-doc. The reason being that there is a small chance
of a name collision if the struct has a field named doc and the
client passes the ?m=all query parameter, which tells godoc to
also show unexported identifiers. This occurs, for example, in the namedType struct of the go/doc pkg.
As for adding links to the struct fields, a good place to start is
the file x/tools/godoc/linkify.go. Specifically the functions
LinkifyText
and
linksFor.
These functions are responsible for (among other things) adding
anchor links for const and var declarations in the current
implementation.

Hope this is helpful.

@appleby

Hi @appleby, thank you for the info, very useful!
I've started working on the links to the struct fields, here's a simple test:
page.html.zip

Have a look at file:///page.html#Test, file:///page.html#Test.What, file:///page.html#Test.Str.
Simply adding an id to the comment span seems enough.

I'll have a look at x/tools/godoc/linkify.go and identify a strategy to automatically add the id.

Please let me know if you guys have any comments or questions.

If you've figured out a way to add the id to the comment span, you should probably just go that route and ignore what I said about LinkifyText. LinkifyText is currently only responsible for adding links that anchor directly to an identifier. For example, when you link to https://golang.org/pkg/net/http/#ErrContentLength, it's anchored at the identifier, not the associated comment block.

I agree that adding the anchor at the top of the comment block is nicer from the user's perspective, but it wasn't clear to me how to get the context required to generate the id at the time the comments are rendered.

One problem with the approach of adding an id attribute to the first comment span tag is that it doesn't scale when multiple fields share the same doc comment. Consider, for example:

type Foo struct {
    // T1, T2, and T3 are templates.
    T1, T2, T3 *text.Template
}

In a case like this, if you go the route of adding an id attribute to the comment tag, you'd have to pick one of the identifiers to make linkable. Alternatively, you could possibly generate separate tags for the anchors prior to writing the comment block.

Another edge-case to keep in mind is fields without an associated doc comment. For example, see the first few fields of the godoc.Presentation struct.

@appleby

@appleby great examples. I'll keep them in mind.
If you can find more of those then please let us know.

@bradfitz

Here's the proposal only for the adding bookmarks to IDENTs (i.e. structs, vars, etc.):
It's my first proposal ever, so any comment or advice is really appreciated.

In LinkifyText (linkify.go) call BookmarkText located in a new file bookmark.go:

bookmarksWriter := BookmarkText(text)
idents := tokenSelection(text, token.IDENT)
comments := tokenSelection(text, token.COMMENT)
FormatSelections(w, text, linkWriter, idents, selectionTag, bookmarksWriter, comments)

Here's its signature:
func BookmarkText(text []byte) (bw BookmarkWriter)

BookmarkText identifies all the IDENTs using a helper function:

func createBookmarks(text []byte) (bookmarkMap map[int]bookmarkList)
createBookmarks parses the source code and identifies every IDENT that needs a bookmark.
If the first line of a comment is found above its relative IDENT, then a bookmark struct is pushed into a map using the comment line number.
e.g.:

      // Comment
      n := 1

If there isn't a comment block above the IDENT, or the comment is on the right-side of the IDENT, then the IDENT line number is used as key.
e.g.:
n := 1 // Comment

At this point in:
func FormatSelections(w io.Writer, text []byte, lw LinkWriter, links Selection, sw SegmentWriter, bw BookmarkWriter, selections ...Selection)

each time a token is processed a call to:
bw(w, lastOffs, start)
is made. The BookmarkWriter function checks if a lastOffs key exists in the bookmarkMap and prints its related bookmark to w.

Here's a test page:
page.html.zip

Try:

file:///page.html#ErrBodyNotAllowed
file:///page.html#SingleBlock.T3
file:///page.html#Test.What

Note: the test page has each main comment above the its relative node as pointed in #16728. To achieve this changes have been made in package.html, such as:

<h2 id="pkg-constants">Constants</h2>
    {{range .}}
        {{comment_html .Doc}}
        <pre>{{node_html $ .Decl true}}</pre>
    {{end}}

Each {{comment_html .Doc}} has been moved above <pre>{{node_html $ .Decl true}}</pre>.

Please let me know if I need to file a formal proposal or this is sufficient.

/cc @bradfitz @broady @shurcooL @appleby @adg

Here's a test page:
page.html.zip

That page had a Uncaught SyntaxError: Unexpected token ; error, and it wasn't being interpreted as UTF-8. I've fixed the JS error and added <meta charset="utf-8">.

I've uploaded it here for easier viewing:

http://instantshare.virtivia.com:27080/1h5htyea6hnw4.html
http://instantshare.virtivia.com:27080/1h5htyea6hnw4.html#ErrBodyNotAllowed
http://instantshare.virtivia.com:27080/1h5htyea6hnw4.html#SingleBlock.T3
http://instantshare.virtivia.com:27080/1h5htyea6hnw4.html#Test.What

I've only looked briefly, but what I can see so far, this looks very reasonable to me.

I agree with what Alan said above. That rationale is very good.

I disagree with Dmitri about the need for links such as #File.io.Closer.

My comment was not about the need for that link, it was about the need for io component if that link were to be included. I think you have described in a good way why the whole thing should not be linked. That's an even better solution.

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

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

I've just committed the changes (5 mod files, 1 new file) and don't why got two different revision number 32153 and 32154. Does anyone know why?
Furthermore I couldn't push some refs:

# git push origin HEAD:refs/for/master
Counting objects: 9, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 1.52 KiB | 0 bytes/s, done.
Total 9 (delta 8), reused 0 (delta 0)
remote: Resolving deltas: 100% (8/8)
remote: Processing changes: new: 1, done    
remote: 
remote: New Changes:
remote:   https://go-review.googlesource.com/32154 x/tools/cmd/godoc: add links to fields
remote: 
To https://go.googlesource.com/tools
 * [new branch]      HEAD -> refs/for/master
Counting objects: 16868, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (5268/5268), done.
Writing objects: 100% (16868/16868), 7.65 MiB | 89.00 KiB/s, done.
Total 16868 (delta 11370), reused 16734 (delta 11255)
remote: Resolving deltas: 100% (11370/11370)
remote: Counting objects: 16718, done
remote: Processing changes: refs: 1, done    
To https://go-review.googlesource.com/go
 ! [remote rejected] HEAD -> refs/for/master (no common ancestry)
error: failed to push some refs to 'https://go-review.googlesource.com/go'

Any ideas?

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

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

godoc: update struct field anchor code

godoc: make struct fields linkable in HTML mode

Navigation Menu

Repository navigation

x/tools/cmd/godoc: enable links to fields of structs #16753

12 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

x/tools/cmd/godoc: enable links to fields of structs #16753

Description

Activity

bradfitz commented on Aug 16, 2016

broady commented on Aug 16, 2016

bradfitz commented on Aug 16, 2016

dmitshur commented on Aug 16, 2016

dmitshur commented on Aug 16, 2016

luigi-riefolo commented on Sep 6, 2016

bradfitz commented on Sep 6, 2016

appleby commented on Sep 6, 2016

luigi-riefolo commented on Sep 6, 2016

appleby commented on Sep 6, 2016

luigi-riefolo commented on Sep 7, 2016

luigi-riefolo commented on Sep 15, 2016

dmitshur commented on Sep 16, 2016

12 remaining items

dmitshur commented on Oct 15, 2016

gopherbot commented on Oct 27, 2016

gopherbot commented on Oct 27, 2016

luigi-riefolo commented on Oct 27, 2016

gopherbot commented on Nov 30, 2016

gopherbot commented on Dec 1, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions