code review 5700043: godoc: don't show testdata directories

godoc: don't show testdata directories

[gri, 2012-02-23 07:50:33]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitzgoog, 2012-02-23 07:52:52]

LGTM

Where is this excluded from?  Can I still link to testdata files if I know
the path?  Will it still be served?


On Thu, Feb 23, 2012 at 6:50 PM, <gri@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> godoc: don't show testdata directories
>
> Please review this at
http://codereview.appspot.com/**5700043/<http://codereview.appspot.com/5700043/>
>
> Affected files:
>  M src/cmd/godoc/dirtrees.go
>
>
> Index: src/cmd/godoc/dirtrees.go
> ==============================**==============================**=======
> --- a/src/cmd/godoc/dirtrees.go
> +++ b/src/cmd/godoc/dirtrees.go
> @@ -17,6 +17,11 @@
>        "strings"
>  )
>
> +// Conventional name for directories containing test data.
> +// Excluded from directory trees.
> +//
> +const testdataDirName = "testdata"
> +
>  type Directory struct {
>        Depth int
>        Path  string // includes Name
> @@ -49,7 +54,7 @@
>  }
>
>  func (b *treeBuilder) newDirTree(fset *token.FileSet, path, name string,
> depth int) *Directory {
> -       if b.pathFilter != nil && !b.pathFilter(path) {
> +       if b.pathFilter != nil && !b.pathFilter(path) || name ==
> testdataDirName {
>                return nil
>        }
>
>
>
>

[dsymonds, 2012-02-23 08:03:33]

Yeah, it would be good if this *only* excluded them from the package
list, and could still be browsed by naming the URL.

I'd also like to use this for the App Engine SDK; any chance you could
turn this constant into a regular expression?


Dave.

[minux1, 2012-02-23 08:50:35]

On Thu, Feb 23, 2012 at 3:52 PM, Brad Fitzpatrick <bradfitz@google.com>wrote:

> LGTM
>
> Where is this excluded from?  Can I still link to testdata files if I know
> the path?  Will it still be served?
>
It won't be served even you know the exact URL.

[minux1, 2012-02-23 08:57:46]

On Thu, Feb 23, 2012 at 4:50 PM, minux <minux.ma@gmail.com> wrote:

> On Thu, Feb 23, 2012 at 3:52 PM, Brad Fitzpatrick <bradfitz@google.com>wrote:
>
>> LGTM
>>
>> Where is this excluded from?  Can I still link to testdata files if I
>> know the path?  Will it still be served?
>>
> It won't be served even you know the exact URL.
>
To be precise, this behavior isn't introduced by this CL solely.
for example:
http://tip.golang.org/cmd/api/testdata/ OK on tip, but no output with this
CL
http://tip.golang.org/pkg/archive/zip/testdata/  no file listing even on
release.r60.3

[rsc, 2012-02-23 15:48:07]

On Thu, Feb 23, 2012 at 03:03, David Symonds <dsymonds@golang.org> wrote:
> I'd also like to use this for the App Engine SDK; any chance you could
> turn this constant into a regular expression?

Here it is: "testdata".

This string is known in multiple tools and not going to change,
any more than .go is going to change as a file suffix.

Russ

[gri, 2012-02-23 20:23:17]

*** Submitted as http://code.google.com/p/go/source/detail?r=cbe17d3ea720 ***

godoc: don't show testdata directories

R=golang-dev, bradfitz, dsymonds, minux.ma, rsc
CC=golang-dev
http://codereview.appspot.com/5700043