code review 4280048: go/scanner: use filepath

go/scanner: use filepath

[brainman, 2011-03-15 04:28:38]

Hello rsc (cc: golang-dev@googlegroups.com),

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

[rsc, 2011-03-15 15:24:52]

R=gri

looks fine but leaving for gri

[gri, 2011-03-15 16:28:42]

http://codereview.appspot.com/4280048/diff/2004/src/pkg/go/scanner/scanner.go
File src/pkg/go/scanner/scanner.go (right):

http://codereview.appspot.com/4280048/diff/2004/src/pkg/go/scanner/scanner.go...
src/pkg/go/scanner/scanner.go:179: // TODO(brainman): Check for : will fail if
filename is like c:\test.go
Fine for now but it would be nice if we could prevent OS-specific path
peculiarities from creeping in everywhere.

Perhaps we can define a regexp in file path that can be used to match against a
path? Then we can use it to extract the path part here.

http://codereview.appspot.com/4280048/diff/2004/src/pkg/go/scanner/scanner_te...
File src/pkg/go/scanner/scanner_test.go (right):

http://codereview.appspot.com/4280048/diff/2004/src/pkg/go/scanner/scanner_te...
src/pkg/go/scanner/scanner_test.go:447: func mkpath(names ...string) string {
This looks too complicated to me. As far as I can tell, there's only one case
where len(names) != 2.

Why not just use filepath.Join directly?

(I guess I don't understand why filepath.Join won't work for the last case
"dir/a/b/c/File1.go" - but even if doesn't, I would just write that one case as
a sequence of Join calls instead).

[brainman, 2011-03-16 02:07:41]

Hello gri (cc: golang-dev@googlegroups.com),

Please take another look.

[brainman, 2011-03-16 02:07:47]

Thank you for review.

http://codereview.appspot.com/4280048/diff/2004/src/pkg/go/scanner/scanner.go
File src/pkg/go/scanner/scanner.go (right):

http://codereview.appspot.com/4280048/diff/2004/src/pkg/go/scanner/scanner.go...
src/pkg/go/scanner/scanner.go:179: // TODO(brainman): Check for : will fail if
filename is like c:\test.go
On 2011/03/15 16:28:42, gri wrote:
> ... it would be nice if we could prevent OS-specific path
> peculiarities from creeping in everywhere.
> 

OK. I've created an issue instead:

http://code.google.com/p/go/issues/detail?id=1614

http://codereview.appspot.com/4280048/diff/2004/src/pkg/go/scanner/scanner_te...
File src/pkg/go/scanner/scanner_test.go (right):

http://codereview.appspot.com/4280048/diff/2004/src/pkg/go/scanner/scanner_te...
src/pkg/go/scanner/scanner_test.go:447: func mkpath(names ...string) string {
On 2011/03/15 16:28:42, gri wrote:
> 
> Why not just use filepath.Join directly?
> 

Why not, indeed. (Now that I looked to see what it does <g>).

[gri, 2011-03-16 17:34:54]

LGTM

[gri, 2011-03-16 17:38:06]

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

go/scanner: use filepath

R=gri
CC=golang-dev
http://codereview.appspot.com/4280048

Committer: Robert Griesemer <gri@golang.org>