code review 9908043: path, path/filepath: Add Name function

path, path/filepath: Add Name function

Name returns the file name minus the extension.
The extension is the suffix beginning at the final dot
in the final slash-separated element of path. If there
is no extension, the same file name is returned.

[Hierro, 2013-05-31 18:53:18]

Hello golang-dev@googlegroups.com,

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

[r, 2013-05-31 19:03:56]

https://codereview.appspot.com/9908043/diff/5001/src/pkg/path/filepath/path.go
File src/pkg/path/filepath/path.go (right):

https://codereview.appspot.com/9908043/diff/5001/src/pkg/path/filepath/path.g...
src/pkg/path/filepath/path.go:227: }
this function generates garbage by calling Ext. it doesn't need to.

also the name isn't great. i think Name might be right - the file is the dir
plus base, and base is name plus ext.

[Hierro, 2013-05-31 19:15:54]

On 2013/05/31 19:03:56, r wrote:
> https://codereview.appspot.com/9908043/diff/5001/src/pkg/path/filepath/path.go
> File src/pkg/path/filepath/path.go (right):
> 
>
https://codereview.appspot.com/9908043/diff/5001/src/pkg/path/filepath/path.g...
> src/pkg/path/filepath/path.go:227: }
> this function generates garbage by calling Ext. it doesn't need to.

Good call. I'll fix that.

> 
> also the name isn't great. i think Name might be right - the file is the dir
> plus base, and base is name plus ext.

Seems like a good name to me. I'll make those two changes and resubmit.

Regards,
Alberto

[Hierro, 2013-05-31 19:20:38]

PTAL

[r, 2013-05-31 19:24:16]

https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/filepath/path.go
File src/pkg/path/filepath/path.go (right):

https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/filepath/path....
src/pkg/path/filepath/path.go:221: // Name returns the file name minus the
extension.
s/name/base name/

https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/filepath/path....
src/pkg/path/filepath/path.go:224: // is no extension, the same file name is
returned.
s/same file/original/

https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/path.go
File src/pkg/path/path.go (right):

https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/path.go#newcod...
src/pkg/path/path.go:176: func Name(path string) string {
ditto

[r, 2013-05-31 19:24:34]

Also, please add an entry to doc/go1.2.txt

[Hierro, 2013-05-31 19:30:06]

On 2013/05/31 19:24:16, r wrote:
>
https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/filepath/path.go
> File src/pkg/path/filepath/path.go (right):
> 
>
https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/filepath/path....
> src/pkg/path/filepath/path.go:221: // Name returns the file name minus the
> extension.
> s/name/base name/

Wouldn't this make people expect Name("foo/bar.h") to return "bar" rather than
"foo/bar", because of the Unix command
basename?

> 
>
https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/filepath/path....
> src/pkg/path/filepath/path.go:224: // is no extension, the same file name is
> returned.
> s/same file/original/

Noted

[r, 2013-06-03 13:48:28]

https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/filepath/path.go
File src/pkg/path/filepath/path.go (right):

https://codereview.appspot.com/9908043/diff/15001/src/pkg/path/filepath/path....
src/pkg/path/filepath/path.go:228: return path[:i]
hold on. just realized because of your answer to my question that this isn't
doing what i want.

given

foo/bar.h

this should return bar but it returns foo/bar.  foo/bar seems occasionally
useful but much less so than the bar.

are we at an impasse?

[rsc, 2013-06-03 18:38:55]

This implementation is wrong for "Name", as Rob points out. And actually there's
no reason why Name should not include the suffix, really.

I suggest not doing anything about this. It's easy to write one of:

    strings.TrimSuffix(p, path.Ext(p))

    base := path.Base(p)
    base = strings.TrimSuffix(base, path.Ext(base))

depending on what you mean.

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

I've thought about it some more and agree, let's let it go. It's not
compelling.

The expression strings.TrimSuffix(base, path.Ext(base)) is efficient
enough for the rare occasions it's needed, and the slice returned by
path.Ext doesn't return garbage (I misspoke earlier).

-rob

[Hierro, 2013-06-03 21:20:05]

On 2013/06/03 18:52:26, r wrote:
> I've thought about it some more and agree, let's let it go. It's not
> compelling.
> 
> The expression strings.TrimSuffix(base, path.Ext(base)) is efficient
> enough for the rare occasions it's needed, and the slice returned by
> path.Ext doesn't return garbage (I misspoke earlier).
> 
> -rob

Good, let's close it then.

[Hierro, 2013-06-03 21:23:52]

*** Abandoned ***