code review 7291043: path/filepath: add examples for SplitList and Rel.

path/filepath: add examples for SplitList and Rel.

[kisielk, 2013-02-03 18:55:57]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2013-02-04 17:27:11]

Would these tests pass on Windows too?

On Sun, Feb 3, 2013 at 10:55 AM, <kamil.kisiel@gmail.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> path/filepath: Added examples for SplitList and Rel.
>
> Please review this at
https://codereview.appspot.**com/7291043/<https://codereview.appspot.com/7291...
>
> Affected files:
>   A src/pkg/path/filepath/example_**test.go
>
>
> Index: src/pkg/path/filepath/example_**test.go
> ==============================**==============================**=======
> new file mode 100644
> --- /dev/null
> +++ b/src/pkg/path/filepath/**example_test.go
> @@ -0,0 +1,34 @@
> +// Copyright 2013 The Go Authors. All rights reserved.
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE file.
> +
> +package filepath_test
> +
> +import (
> +       "fmt"
> +       "path/filepath"
> +)
> +
> +func ExampleSplitList() {
> +       fmt.Println(filepath.**SplitList("/a/b/c:/usr/bin"))
> +       // Output: [/a/b/c /usr/bin]
> +}
> +
> +func ExampleRel() {
> +       paths := []string{
> +               "/a/b/c",
> +               "/b/c",
> +               "./b/c",
> +       }
> +       base := "/a"
> +
> +       for _, p := range paths {
> +               rel, err := filepath.Rel(base, p)
> +               fmt.Printf("%q: %q %v\n", p, rel, err)
> +       }
> +
> +       // Output:
> +       // "/a/b/c": "b/c" <nil>
> +       // "/b/c": "../b/c" <nil>
> +       // "./b/c": "" Rel: can't make b/c relative to /a
> +}
>
>
> --
>
> ---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...
> .
>
>
>

[minux1, 2013-02-04 17:29:37]

On Tue, Feb 5, 2013 at 1:27 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> Would these tests pass on Windows too?

it won't.

=== RUN: ExampleRel
--- FAIL: ExampleRel (2.0001ms)
got:
"/a/b/c": "b\\c" <nil>
"/b/c": "..\\b\\c" <nil>
"./b/c": "" Rel: can't make b\c relative to \a
want:
"/a/b/c": "b/c" <nil>
"/b/c": "../b/c" <nil>
"./b/c": "" Rel: can't make b/c relative to /a
=== RUN: ExampleSplitList
--- FAIL: ExampleSplitList (0)
got:
[/a/b/c:/usr/bin]
want:
[/a/b/c /usr/bin]
FAIL
exit status 1

[bradfitz, 2013-02-04 17:31:45]

On Mon, Feb 4, 2013 at 9:29 AM, minux <minux.ma@gmail.com> wrote:

>
> On Tue, Feb 5, 2013 at 1:27 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:
>
>> Would these tests pass on Windows too?
>
> it won't.
>

That's what I suspected.  That makes writing filepath example docs harder.

Perhaps we need to +build two example files to windows and !windows?

[minux1, 2013-02-04 17:48:13]

On Tue, Feb 5, 2013 at 1:31 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> On Mon, Feb 4, 2013 at 9:29 AM, minux <minux.ma@gmail.com> wrote:
>
>>
>> On Tue, Feb 5, 2013 at 1:27 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:
>>
>>> Would these tests pass on Windows too?
>>
>> it won't.
>>
>
> That's what I suspected.  That makes writing filepath example docs harder.
>
>  Perhaps we need to +build two example files to windows and !windows?
>
How about using ToSlash() to convert the output to canonical path before
output,
and mention that the path generated will be using native path separator in
comment?

[kisielk, 2013-02-04 17:58:00]

On Mon, Feb 4, 2013 at 9:47 AM, minux <minux.ma@gmail.com> wrote:

>
> On Tue, Feb 5, 2013 at 1:31 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:
>
>> On Mon, Feb 4, 2013 at 9:29 AM, minux <minux.ma@gmail.com> wrote:
>>
>>>
>>> On Tue, Feb 5, 2013 at 1:27 AM, Brad Fitzpatrick
<bradfitz@golang.org>wrote:
>>>
>>>> Would these tests pass on Windows too?
>>>
>>> it won't.
>>>
>>
>> That's what I suspected.  That makes writing filepath example docs harder.
>>
>>  Perhaps we need to +build two example files to windows and !windows?
>>
> How about using ToSlash() to convert the output to canonical path before
> output,
> and mention that the path generated will be using native path separator in
> comment?
>
>
I'm not sure that would be better than having no example, it would not be
nearly as clear.
That also doesn't help with the example for SplitList.

I like Brad's suggestion, though I don't have a Windows machine to test on
at the moment.

[bradfitz, 2013-02-04 18:24:16]

If you don't have access to Windows, just do one file for now, with:

// +build !windows,!plan9



And others can do Windows and Plan 9 later.


On Mon, Feb 4, 2013 at 9:57 AM, Kamil Kisiel <kamil.kisiel@gmail.com> wrote:

> On Mon, Feb 4, 2013 at 9:47 AM, minux <minux.ma@gmail.com> wrote:
>
>>
>> On Tue, Feb 5, 2013 at 1:31 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:
>>
>>> On Mon, Feb 4, 2013 at 9:29 AM, minux <minux.ma@gmail.com> wrote:
>>>
>>>>
>>>> On Tue, Feb 5, 2013 at 1:27 AM, Brad Fitzpatrick
<bradfitz@golang.org>wrote:
>>>>
>>>>> Would these tests pass on Windows too?
>>>>
>>>> it won't.
>>>>
>>>
>>> That's what I suspected.  That makes writing filepath example docs
>>> harder.
>>>
>>>  Perhaps we need to +build two example files to windows and !windows?
>>>
>> How about using ToSlash() to convert the output to canonical path before
>> output,
>> and mention that the path generated will be using native path separator
>> in comment?
>>
>>
> I'm not sure that would be better than having no example, it would not be
> nearly as clear.
> That also doesn't help with the example for SplitList.
>
> I like Brad's suggestion, though I don't have a Windows machine to test on
> at the moment.
>

[kisielk, 2013-02-06 03:40:32]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[minux1, 2013-02-06 10:51:19]

https://codereview.appspot.com/7291043/diff/12001/src/pkg/path/filepath/examp...
File src/pkg/path/filepath/example_test.go (right):

https://codereview.appspot.com/7291043/diff/12001/src/pkg/path/filepath/examp...
src/pkg/path/filepath/example_test.go:5: // +build !windows,!plan9
if we do this, i think it's better to rename the file to
example_unix_test.go
so that latter someone could write corresponding
example_{plan9,windows}_test.go

[kisielk, 2013-02-06 16:25:49]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[minux1, 2013-02-06 17:10:35]

LGTM. brad? others?

[bradfitz, 2013-02-06 19:33:28]

I'm concerned that windows users reading the docs online will be misled.

Maybe the example output should say "output on a Unix system:" or something.
 On Feb 6, 2013 9:10 AM, <minux.ma@gmail.com> wrote:

> LGTM. brad? others?
>
>
https://codereview.appspot.**com/7291043/<https://codereview.appspot.com/7291...
>

[minux1, 2013-02-06 19:41:01]

On Thursday, February 7, 2013, Brad Fitzpatrick wrote:

> I'm concerned that windows users reading the docs online will be misled.
>
> Maybe the example output should say "output on a Unix system:" or
> something.
>
can we do something magical like this in cmd/godoc?
1. provide a way to override GOOS for online godoc (?os=windows)
2. infer that from user-agent string/javascript (if deployed on GAE)

1 alone is pretty useful.

[kisielk, 2013-02-06 21:07:31]

On Wed, Feb 6, 2013 at 11:41 AM, minux <minux.ma@gmail.com> wrote:

>
> On Thursday, February 7, 2013, Brad Fitzpatrick wrote:
>
>> I'm concerned that windows users reading the docs online will be misled.
>>
>> Maybe the example output should say "output on a Unix system:" or
>> something.
>>
> can we do something magical like this in cmd/godoc?
> 1. provide a way to override GOOS for online godoc (?os=windows)
> 2. infer that from user-agent string/javascript (if deployed on GAE)
>
> 1 alone is pretty useful.
>

I agree that it would be useful to be able to browse different versions of
the docs online, but it would be useful to have some way to switch between
them in the interface.
The device someone is browsing the documentation on is not necessarily the
same as the one they are developing for.

I think for this CL Brad's suggestion makes sense and I can do that if
others agree.

[bradfitz, 2013-02-09 01:33:22]

Go for it.  Perfect is the enemy of good, and all that.

I'm not actually so keen on user-agent sniffing to show the "right" docs.
 Seems too tricky and prone to confuse people more.

Let's just do the simple thing.


On Wed, Feb 6, 2013 at 1:06 PM, Kamil Kisiel <kamil.kisiel@gmail.com> wrote:

> On Wed, Feb 6, 2013 at 11:41 AM, minux <minux.ma@gmail.com> wrote:
>
>>
>> On Thursday, February 7, 2013, Brad Fitzpatrick wrote:
>>
>>> I'm concerned that windows users reading the docs online will be misled.
>>>
>>> Maybe the example output should say "output on a Unix system:" or
>>> something.
>>>
>> can we do something magical like this in cmd/godoc?
>> 1. provide a way to override GOOS for online godoc (?os=windows)
>> 2. infer that from user-agent string/javascript (if deployed on GAE)
>>
>> 1 alone is pretty useful.
>>
>
> I agree that it would be useful to be able to browse different versions of
> the docs online, but it would be useful to have some way to switch between
> them in the interface.
> The device someone is browsing the documentation on is not necessarily the
> same as the one they are developing for.
>
> I think for this CL Brad's suggestion makes sense and I can do that if
> others agree.
>

[kisielk, 2013-02-13 04:17:22]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[minux1, 2013-02-13 16:08:41]

please don't change the "// Output:" line, or your example won't
be run by "go test".

https://codereview.appspot.com/7291043/diff/24001/src/pkg/path/filepath/examp...
File src/pkg/path/filepath/example_unix_test.go (right):

https://codereview.appspot.com/7291043/diff/24001/src/pkg/path/filepath/examp...
src/pkg/path/filepath/example_unix_test.go:16: // Output on a Unix system:
func ExampleSplitList() {
    fmt.Println("On Unix:", filepath.SplitList("/a/b/c:/usr/bin"))
    // Output:
    // On Unix: [/a/b/c /usr/bin]
}

https://codereview.appspot.com/7291043/diff/24001/src/pkg/path/filepath/examp...
src/pkg/path/filepath/example_unix_test.go:33: // Output on a Unix system:
    fmt.Println("On Unix:")
    for _, p := range paths {
        rel, err := filepath.Rel(base, p)
        fmt.Printf("%q: %q %v\n", p, rel, err)
    }   

    // Output:
    // On Unix:
    // "/a/b/c": "b/c" <nil>
    // "/b/c": "../b/c" <nil>
    // "./b/c": "" Rel: can't make b/c relative to /a

[kisielk, 2013-02-14 07:35:29]

Of course, I should have used go test -v to double-check before submitting the
last one. Sorry.

PTAL

[minux1, 2013-02-14 08:00:45]

LGTM. Leave for Brad.

[bradfitz, 2013-02-19 18:41:04]

LGTM

[bradfitz, 2013-02-19 18:41:39]

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

path/filepath: add examples for SplitList and Rel.

R=golang-dev, bradfitz, minux.ma
CC=golang-dev
https://codereview.appspot.com/7291043

Committer: Brad Fitzpatrick <bradfitz@golang.org>

[kisielk, 2013-04-03 04:49:06]

*** Abandoned ***