code review 7883044: godoc: link identifiers to declarations

godoc: link identifiers to declarations

The changes are almost completely self-contained
in the new file linkify.go. The other changes are
minimal and should not disturb the currently
working godoc, in anticipation of Go 1.1.

To disable the feature in case of problems, set
-links=false.

Fixes issue 2063.

[gri, 2013-03-26 00:03:48]

Hello adg@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[gri, 2013-03-26 05:01:22]

Hello adg@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[r, 2013-03-26 17:54:34]

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/format.go
File src/cmd/godoc/format.go (right):

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/format.go#new...
src/cmd/godoc/format.go:230: // of token sel in the Go src text as a Selection.
tokenSelection returns, as a selection, the sequence...

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/godoc.go#newc...
src/cmd/godoc/godoc.go:1046: pkg = ast.NewObj(ast.Pkg,
path[strings.LastIndex(path, "/")+1:])
tricky how this works if there's no slash, but OK

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/linkify.go
File src/cmd/godoc/linkify.go (right):

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/linkify.go#ne...
src/cmd/godoc/linkify.go:92: // Append a "no link".
i can fix that in a heartbeat so delete this
https://codereview.appspot.com/7992044

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/linkify.go#ne...
src/cmd/godoc/linkify.go:201: "nil":        true, // not in the builtins package
- see special case in links function
s/builtins/builtin/g
but delete because of https://codereview.appspot.com/7992044

[gri, 2013-03-26 18:07:52]

Hello adg@golang.org, r@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[r, 2013-03-26 18:12:52]

LGTM

[gri, 2013-03-26 18:14:33]

*** Submitted as https://code.google.com/p/go/source/detail?r=495b88abe2b2 ***

godoc: link identifiers to declarations

The changes are almost completely self-contained
in the new file linkify.go. The other changes are
minimal and should not disturb the currently
working godoc, in anticipation of Go 1.1.

To disable the feature in case of problems, set
-links=false.

Fixes issue 2063.

R=adg, r
CC=golang-dev
https://codereview.appspot.com/7883044

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/format.go
File src/cmd/godoc/format.go (right):

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/format.go#new...
src/cmd/godoc/format.go:230: // of token sel in the Go src text as a Selection.
On 2013/03/26 17:54:34, r wrote:
> tokenSelection returns, as a selection, the sequence...

Done.

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/godoc.go
File src/cmd/godoc/godoc.go (right):

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/godoc.go#newc...
src/cmd/godoc/godoc.go:1046: pkg = ast.NewObj(ast.Pkg,
path[strings.LastIndex(path, "/")+1:])
On 2013/03/26 17:54:34, r wrote:
> tricky how this works if there's no slash, but OK

added a comment just to be clear

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/linkify.go
File src/cmd/godoc/linkify.go (right):

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/linkify.go#ne...
src/cmd/godoc/linkify.go:92: // Append a "no link".
On 2013/03/26 17:54:34, r wrote:
> i can fix that in a heartbeat so delete this
> https://codereview.appspot.com/7992044

Done.

https://codereview.appspot.com/7883044/diff/30001/src/cmd/godoc/linkify.go#ne...
src/cmd/godoc/linkify.go:201: "nil":        true, // not in the builtins package
- see special case in links function
On 2013/03/26 17:54:34, r wrote:
> s/builtins/builtin/g
> but delete because of https://codereview.appspot.com/7992044

Done.

[adg, 2013-03-27 03:38:50]

https://codereview.appspot.com/7883044/diff/43001/src/cmd/godoc/linkify.go
File src/cmd/godoc/linkify.go (right):

https://codereview.appspot.com/7883044/diff/43001/src/cmd/godoc/linkify.go#ne...
src/cmd/godoc/linkify.go:170: var predeclared = map[string]bool{
This map is already defined internally in go/doc. Do they match?

[gri, 2013-03-27 04:32:59]

https://codereview.appspot.com/7883044/diff/43001/src/cmd/godoc/linkify.go
File src/cmd/godoc/linkify.go (right):

https://codereview.appspot.com/7883044/diff/43001/src/cmd/godoc/linkify.go#ne...
src/cmd/godoc/linkify.go:170: var predeclared = map[string]bool{
On 2013/03/27 03:38:51, adg wrote:
> This map is already defined internally in go/doc. Do they match?

go/doc has three separate maps but together they should match. Haven't made up
my mind as to whether I should export a function (isBuiltinName or some such).
I'll have a look again.