code review 9722045: cmd/godoc: don't link unexported identifiers

cmd/godoc: don't link unexported identifiers

[titanous, 2013-05-27 19:10:46]

Hello golang-dev@googlegroups.com,

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

[gri1, 2013-05-27 21:21:47]

Offline today (memorial day) but just noticed this one: I think ast is
already imported and there is an IsExported function and method. Please use
those instead. Thanks.
-gri
On May 27, 2013 12:10 PM, <jonathan@titanous.com> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> cmd/godoc: don't link unexported identifiers
>
> Please review this at
https://codereview.appspot.**com/9722045/<https://codereview.appspot.com/9722...
>
> Affected files:
>   M src/cmd/godoc/linkify.go
>
>
> Index: src/cmd/godoc/linkify.go
> ==============================**==============================**=======
> --- a/src/cmd/godoc/linkify.go
> +++ b/src/cmd/godoc/linkify.go
> @@ -16,6 +16,8 @@
>         "go/token"
>         "io"
>         "strconv"
> +       "unicode"
> +       "unicode/utf8"
>  )
>
>  // LinkifyText HTML-escapes source text and writes it to w.
> @@ -56,7 +58,7 @@
>                                 if info.mode == identVal {
>                                         fmt.Fprintf(w, `<span id="%s">`,
> info.name)
>                                         prev = "span"
> -                               } else {
> +                               } else if isExported(info.name) {
>                                         fmt.Fprintf(w, `<a href="#%s">`,
> info.name)
>                                         prev = "a"
>                                 }
> @@ -187,6 +189,12 @@
>         return m
>  }
>
> +// isExported returns true if the name is exported
> +func isExported(name string) bool {
> +       r, _ := utf8.DecodeRuneInString(name)
> +       return unicode.IsUpper(r)
> +}
> +
>  // The predeclared map represents the set of all predeclared identifiers.
>  // TODO(gri) This information is also encoded in similar maps in go/doc,
>  //           but not exported. Consider exporting an accessor and using
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[titanous, 2013-05-28 01:28:50]

On 2013/05/27 21:21:47, gri1 wrote:
> Offline today (memorial day) but just noticed this one: I think ast is
> already imported and there is an IsExported function and method. Please use
> those instead. Thanks.
> -gri

Done.

[titanous, 2013-06-14 17:17:30]

PTAL. This is a one-line change.

[gri, 2013-06-14 19:30:47]

LGTM

But before I submit this, do you have a concrete case (link, test, what have
you) where I can see the effect? Thanks.
- gri

[titanous, 2013-06-14 19:32:49]

On 2013/06/14 19:30:47, gri wrote:
> LGTM
> 
> But before I submit this, do you have a concrete case (link, test, what have
> you) where I can see the effect? Thanks.
> - gri

http://golang.org/pkg/encoding/xml/#pkg-variables
http://golang.org/pkg/encoding/base64/#pkg-variables

[gri1, 2013-06-14 19:37:10]

Excellent. Thanks.
- gri


On Fri, Jun 14, 2013 at 12:32 PM, <jonathan@titanous.com> wrote:

> On 2013/06/14 19:30:47, gri wrote:
>
>> LGTM
>>
>
>  But before I submit this, do you have a concrete case (link, test,
>>
> what have
>
>> you) where I can see the effect? Thanks.
>> - gri
>>
>
>
http://golang.org/pkg/**encoding/xml/#pkg-variables<http://golang.org/pkg/enc...
>
http://golang.org/pkg/**encoding/base64/#pkg-variables<http://golang.org/pkg/...
>
>
https://codereview.appspot.**com/9722045/<https://codereview.appspot.com/9722...
>

[gri, 2013-06-14 19:37:26]

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

cmd/godoc: don't link unexported identifiers

R=golang-dev, gri, gri
CC=golang-dev
https://codereview.appspot.com/9722045

Committer: Robert Griesemer <gri@golang.org>