Make dirname and basename ignore trailing separators and double slashes.

Make dirname and basename ignore trailing separators and double slashes.

Separators are still preserved for other path methods.

Committed: https://code.google.com/p/dart/source/detail?r=16397

[Bob Nystrom, 2012-12-20 02:27:28]

Two suggestions but LGTM.

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart
File pkg/path/lib/path.dart (right):

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart#newcod...
pkg/path/lib/path.dart:239: parsed.removeTrailingSeparators();
Is this redundant with the above call to removeTrailingSeparators()?

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart#newcod...
pkg/path/lib/path.dart:477: for (var i = parsed.parts.length - 1; i >= 0; i--) {
This is pretty hairy. One option would to have _ParsedPath have a separate field
for the trailing slashes (if any) instead of having a bunch of empty parts.
Parsing gets a bit more complicated, but it localizes that complexity there.

Thoughts?

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart#newcod...
pkg/path/lib/path.dart:679: _ParsedPath copy() => new _ParsedPath(
Nit, but I would prefer "clone" for this.

https://codereview.chromium.org/11638021/diff/1/pkg/path/test/path_posix_test...
File pkg/path/test/path_posix_test.dart (right):

https://codereview.chromium.org/11638021/diff/1/pkg/path/test/path_posix_test...
pkg/path/test/path_posix_test.dart:372:
expect(builder.withoutExtension('a/b.c//'), 'a/b//');
Nice.

[nweiz, 2012-12-20 20:54:49]

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart
File pkg/path/lib/path.dart (right):

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart#newcod...
pkg/path/lib/path.dart:239: parsed.removeTrailingSeparators();
On 2012/12/20 02:27:28, Bob Nystrom wrote:
> Is this redundant with the above call to removeTrailingSeparators()?

No. The first call removes the separators at the end of the input path
("path///to///" -> "path///to"), the second call removes the separators at the
end of the output path ("path///" -> "path").

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart#newcod...
pkg/path/lib/path.dart:477: for (var i = parsed.parts.length - 1; i >= 0; i--) {
On 2012/12/20 02:27:28, Bob Nystrom wrote:
> This is pretty hairy. One option would to have _ParsedPath have a separate
field
> for the trailing slashes (if any) instead of having a bunch of empty parts.
> Parsing gets a bit more complicated, but it localizes that complexity there.
> 
> Thoughts?

It's not just trailing slashes that cause empty components; any path with
multiple slashes anywhere will have empty components. Leaving the trailing
slashes as empty components at least makes the handling uniform between those
two cases.

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart#newcod...
pkg/path/lib/path.dart:679: _ParsedPath copy() => new _ParsedPath(
On 2012/12/20 02:27:28, Bob Nystrom wrote:
> Nit, but I would prefer "clone" for this.

Done.

https://codereview.chromium.org/11638021/diff/1/pkg/path/test/path_posix_test...
File pkg/path/test/path_posix_test.dart (right):

https://codereview.chromium.org/11638021/diff/1/pkg/path/test/path_posix_test...
pkg/path/test/path_posix_test.dart:372:
expect(builder.withoutExtension('a/b.c//'), 'a/b//');
On 2012/12/20 02:27:28, Bob Nystrom wrote:
> Nice.

Just for you.

[Bob Nystrom, 2012-12-20 21:32:57]

LGTM!

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart
File pkg/path/lib/path.dart (right):

https://codereview.chromium.org/11638021/diff/1/pkg/path/lib/path.dart#newcod...
pkg/path/lib/path.dart:477: for (var i = parsed.parts.length - 1; i >= 0; i--) {
On 2012/12/20 20:54:49, nweiz wrote:
> On 2012/12/20 02:27:28, Bob Nystrom wrote:
> > This is pretty hairy. One option would to have _ParsedPath have a separate
> field
> > for the trailing slashes (if any) instead of having a bunch of empty parts.
> > Parsing gets a bit more complicated, but it localizes that complexity there.
> > 
> > Thoughts?
> 
> It's not just trailing slashes that cause empty components; any path with
> multiple slashes anywhere will have empty components. Leaving the trailing
> slashes as empty components at least makes the handling uniform between those
> two cases.

Ah, right.