code review 132730043: path, path/filepath: remove dead code

path, path/filepath: remove dead code

Fixes issue 8503.

Thanks to no.smile.face for the original report.

[dave_cheney.net, 2014-08-21 09:44:29]

Hello bradfitz@golang.org, ruiu@google.com (cc:
golang-codereviews@googlegroups.com),

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

[bradfitz, 2014-08-21 17:28:03]

LGTM

But cc Rob in case he remembers why this was written like this.



On Thu, Aug 21, 2014 at 2:44 AM, <dave@cheney.net> wrote:

> Reviewers: bradfitz, ruiu,
>
> Message:
> Hello bradfitz@golang.org, ruiu@google.com (cc:
> golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> path, path/filepath: remove dead code
>
> Fixes issue 8503.
>
> Thanks to no.smile.face for the original report.
>
> Please review this at https://codereview.appspot.com/132730043/
>
> Affected files (+1, -16 lines):
>   M src/pkg/path/filepath/path.go
>   M src/pkg/path/path.go
>
>
> Index: src/pkg/path/filepath/path.go
> ===================================================================
> --- a/src/pkg/path/filepath/path.go
> +++ b/src/pkg/path/filepath/path.go
> @@ -448,13 +448,6 @@
>                 i--
>         }
>         dir := Clean(path[len(vol) : i+1])
> -       last := len(dir) - 1
> -       if last > 0 && os.IsPathSeparator(dir[last]) {
> -               dir = dir[:last]
> -       }
> -       if dir == "" {
> -               dir = "."
> -       }
>         return vol + dir
>  }
>
> Index: src/pkg/path/path.go
> ===================================================================
> --- a/src/pkg/path/path.go
> +++ b/src/pkg/path/path.go
> @@ -206,13 +206,5 @@
>  // slash.
>  func Dir(path string) string {
>         dir, _ := Split(path)
> -       dir = Clean(dir)
> -       last := len(dir) - 1
> -       if last > 0 && dir[last] == '/' {
> -               dir = dir[:last]
> -       }
> -       if dir == "" {
> -               dir = "."
> -       }
> -       return dir
> +       return Clean(dir)
>  }
>
>
>

[r, 2014-08-21 17:35:29]

LGTM

probably a historical artifact.

[ruiu, 2014-08-21 20:15:12]

https://codereview.appspot.com/132730043/diff/60001/src/pkg/path/filepath/pat...
File src/pkg/path/filepath/path.go (right):

https://codereview.appspot.com/132730043/diff/60001/src/pkg/path/filepath/pat...
src/pkg/path/filepath/path.go:447: for i >= len(vol) &&
!os.IsPathSeparator(path[i]) {
Isn't the for loop also dead code? The loop seems to eliminate trailing path
separators, but Clean also does that, so we don't need to do that here.

[r, 2014-08-21 20:21:03]

https://codereview.appspot.com/132730043/diff/60001/src/pkg/path/filepath/pat...
File src/pkg/path/filepath/path.go (right):

https://codereview.appspot.com/132730043/diff/60001/src/pkg/path/filepath/pat...
src/pkg/path/filepath/path.go:447: for i >= len(vol) &&
!os.IsPathSeparator(path[i]) {
On 2014/08/21 20:15:12, ruiu wrote:
> Isn't the for loop also dead code? The loop seems to eliminate trailing path
> separators, but Clean also does that, so we don't need to do that here.

That's not what the loop does. It does the opposite.
Its purpose is to find the final path separator, the one between the directory
name and the final path element.

[ruiu, 2014-08-21 20:27:59]

LGTM

https://codereview.appspot.com/132730043/diff/60001/src/pkg/path/filepath/pat...
File src/pkg/path/filepath/path.go (right):

https://codereview.appspot.com/132730043/diff/60001/src/pkg/path/filepath/pat...
src/pkg/path/filepath/path.go:447: for i >= len(vol) &&
!os.IsPathSeparator(path[i]) {
Ah, I clearly misunderstood the code.

[dave_cheney.net, 2014-08-22 12:22:02]

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

path, path/filepath: remove dead code

Fixes issue 8503.

Thanks to no.smile.face for the original report.

LGTM=bradfitz, r, ruiu
R=bradfitz, ruiu, r
CC=golang-codereviews
https://codereview.appspot.com/132730043